Bugzilla – Full Text Bug Listing
| Summary: | [parser] Modification to support IMP and exploit LPG 2 | ||
|---|---|---|---|
| Product: | [Modeling] OCL | Reporter: | Ed Willink <ed> |
| Component: | Core | Assignee: | Adolfo Sanchez-Barbudo Herrera <adolfosbh> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | adolfosbh, alexander.igdalov, Ed.Merks, give.a.damus, laurent.goubet, q5n.glineur, rfuhrer, serg.boyko2011 |
| Version: | 1.2.0 | Keywords: | plan |
| Target Milestone: | 3.0.0 | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | Release Currency | ||
| Bug Depends on: | 288666, 298542, 298562 | ||
| Bug Blocks: | 297966, 298634 | ||
| Attachments: | |||
Great work !!! Having the possibility of (auto-)generating editors and related features seems to be a good step for OCL(-Tools?¿), and extending languages such as QVT, even if we don't take advantages of the possible new features which LPG 2.x could provide. I'll start to make some experiments around this topic. Thank you very much Ed for this helpful insight ;) Cheers, Adolfo. Adding Robert Fuhrer in the hope that the 'experience report' can be appropriately triaged as IMP bugs, LPG 2 bugs, user stupidity. Adding to the 2.0 plan, assigning to Ed to contribute when the LPG/IMP problems are resolved. It seems that LPG 2.0 is not API-compatible with 1.x, as the package name is different. So, this bug would have to target the 2.0 release. I'd like to avoid breaking API changes, unless the OCL RTF can release a 2.1 rev this year that fixes some major spec problems (which will necessarily be API-breaking). To avoid a breaking API change, I am currently realising CSTNode.end/startToken by attaching an eAdapter to the CSTNode. This is a bit klunky and inefficient, but allows Ganymede MDT OCL to be used as-is.
It's just necessary for an XXXAnalyzer to construct a DerivedXxxParser that overrides each setOffset to delegate to the CSTNodeTokenAdapter whose static helper functionality orchestrates the dynamic adapter.
dotArrowExpCS ::= NUMERIC_OPERATION '(' argumentsCSopt ')'
is still a problem though.
I am maintaining a patch against 1.2.0 for this enhancement at /cvsroot/modeling org.eclipse.m2m/org.eclipse.qvt.declarative/plugins /org.eclipse.qvt.declarative.parser/model/OCLCST.patch In addition to CSTNode.startToken, CSTNode.endToken, I have added CSTNode.ast, the inverse of the initASTMapping(). The simple fix for bug 243976 is also included. The patch changes the version to 1.2.90.qualifier, which reflects its API compatibility with 1.2.0 in all respects except the CST. Normal plugins that just demand 1.2.x can use either. Plugins that care about the CST must explicitly specify 1.2.0 or 1.2.90. Created attachment 113562 [details] ManuallyTrimmed Patch wrt Ganymede Source I found that addition of an astNode in each CSTNode was helpful as well. Otherwise the attached patch is as described earlier. So the API break is: Lose CSTNode.startOffset/endOffset. Add CSTNode.startToken/endToken/astNode. Detailed error messages regarding Numeric operations may exhibit reduced column number resolution. Are you able to incorporate this change in Galileo? When you're ready, I can provide a coherent patch for this bug and bug 243976 against current CVS. (In reply to comment #6) > Created an attachment (id=113562) [details] > ManuallyTrimmed Patch wrt Ganymede Source > > I found that addition of an astNode in each CSTNode was helpful as well. > Otherwise the attached patch is as described earlier. So the API break is: > > Lose CSTNode.startOffset/endOffset. Add CSTNode.startToken/endToken/astNode. I think that in this case, as in bug 243976, we can avoid API breakage by retaining the startOffset and endOffset. They can be updated automatically whenever the start and end tokens are set. I am a little concerned about the IToken data type. It isn't serializable, so I'm not sure how persistence of the CST is going to work. > Detailed error messages regarding Numeric operations may exhibit reduced column > number resolution. If we don't eliminate the offsets, then we don't lose that. :-) > Are you able to incorporate this change in Galileo? It will be much easier to do that if we go a little out of our way to maintain compatibility. > When you're ready, I can provide a coherent patch for this bug and bug 243976 > against current CVS. Great! > I am a little concerned about the IToken data type. It isn't
> serializable, so
> I'm not sure how persistence of the CST is going to work.
Good, if somewhat pedantic, point. No one can have actually successfully
saved and loaded a CST model anyway.
I'll make startToken, endToken, astNode transient and if serialisation
is important may be have to keep the bloat to provide something serialisable.
(In reply to comment #8) > > I am a little concerned about the IToken data type. It isn't > > serializable, so > > I'm not sure how persistence of the CST is going to work. > > Good, if somewhat pedantic, point. No one can have actually successfully > saved and loaded a CST model anyway. Well, they could. It would have required a bit of fiddling to find containers for some objects (possibly the Resource, itself). I'm practising my pedanticalness for OMG's benefit. :-) > I'll make startToken, endToken, astNode transient and if serialisation > is important may be have to keep the bloat to provide something serialisable. Thanks. I see in the patch that we're not looking to adopt LPG 2 at this point, which is good from the API perspective. However, in order to schedule this patch into the 1.3 release, we'll have to separate it from the IMP/LPG 2 mandate of this bug. How about moving it over to bug 243976? This bug started largely as an experience recommending deferring LPG 2 migration, until LPG 2 responds to the minor problems. I've got my Ganymede-based code successfully using a latest CVS-based OCL, so I hope to submit the patch for this as part of bug 243976 this afternoon. This should allow us to redistribute a standard OCL 1.3.0Mx for use by early QVT Declarative users on Ganymede. Adolfo: After rereading it appears that the MDT-OCL changes to support IMP were done, but LPG-2 was backed out because I could get $ replaced and include files to work. If you can convert EssentialOCL.g to EssentialOCL.gi etc, then I think we could now be go for LPG 2 in 2.0.0. [Trimming startOffset/endOffset is something we could consider when we discuss API rationalisation.] The keynote at Tools/SC this morning had the not particularly inspiring title of "Language Design for Meta-Programming of Software Composition". It was really mind blowing. It described rASCAL, which simplistically is a Java extension (integrated in Eclipse) that raises all sorts of concepts to first class. The parse_tree type supports direct definition of grammars so that a grammar can be used to define a parser, a parse tree, used for object construction, object pattern matching and visitor action control. The underlying formalism is SDF which is being used by IMP; there is a strong tie between this and IMP. I think we could be looking at more than a ten-fold LOC reduction for the parser and a two-fold reduction for the analyser. rASCAL will not reach release quality for Helios, so while it might be a lot of fun to play with its current almost-alpha release, we cannot exploit it yet. Post-Helios, I would hope to replace LPG, get the lexer and CST models auto-defined and parsed, and the CST to AST conversion dramatically simplified. Consequently, I would discourage expending effort converting to LPG 2 now since we may want to ditch LPG altogether. [Adolfo: Change the target to 3.0.0 if you agree.] (In reply to comment #12) > The keynote at Tools/SC this morning had the not particularly inspiring title > of "Language Design for Meta-Programming of Software Composition". > > It was really mind blowing. It described rASCAL, which simplistically is a Java > extension (integrated in Eclipse) that raises all sorts of concepts to first > class. The parse_tree type supports direct definition of grammars so that a > grammar can be used to define a parser, a parse tree, used for object > construction, object pattern matching and visitor action control. The > underlying formalism is SDF which is being used by IMP; there is a strong tie > between this and IMP. I think we could be looking at more than a ten-fold LOC > reduction for the parser and a two-fold reduction for the analyser. > > rASCAL will not reach release quality for Helios, so while it might be a lot of > fun to play with its current almost-alpha release, we cannot exploit it yet. > Post-Helios, I would hope to replace LPG, get the lexer and CST models > auto-defined and parsed, and the CST to AST conversion dramatically simplified. > > Consequently, I would discourage expending effort converting to LPG 2 now since > we may want to ditch LPG altogether. > > [Adolfo: Change the target to 3.0.0 if you agree.] > Hi Ed, I'm still on the idea that the benefits (in performance impact terms) of using the LPG 2 backtracking parser vs the efforts needed to convert the grammars are considerable. Another point is that if we are finally going to include IMP code for the editors, I guess we will need LPG 2. Are we going to ship both LPG plugins in MDT-OCL 2.0 ? I'm inclined on investing some hours in making this conversion. Since I have an OCL grammar extension, I could also oversee any problem with the extending languages. Cheers, Adolfo. Adolfo: I was discouraging not forbidding an upgrade. If you want to do the upgrade assign the bug to yourself. It might be appropriate to wait till we have resolved all MDT-OCL 1.4.0/2.0.0/3.0.0 discussions. LPG 2 is not in Orbit. One of Christian's roles was LPG support for Orbit, and he was planning to add LPG 2 to Orbit. Adolfo: You might care to join Orbit in order to contribute LPG 2. Well, I know what Orbit is but I'm not sure what I should do. I'm sending an email to Orbit-dev list to ask about information.... I assign to myself the bug... Cheers, Adolfo. Hi all, I have created a KeywordTemplateD.gi, KwLexerMap.gi, LexerBasicMap.gi, LexerTemplateD.gi, EsentialOCL.gi, OCLKWLexer.gi, OCLLexer.gi and finally a new version of OCLParser.g so that they are successfully swallowed by lpg.exe v 2.0.17 (the last LPG version in sourceforge). However 2 main problems arose: - As Ed noticed in this post https://sourceforge.net/forum/forum.php?thread_id=2132962&forum_id=523519 the OCLKWLexer missed the $ charactef of the tokenKind array. I have only been able to make the $ appear if I remove the $Id$ from the %Notice section :( - The second problem was a little bit annoying. It seems that with this LPG version you don't have to state /. $BeginJava ... $EndJava ./ after every production rule. You can actually define how $BeginJava and $EndJava macros expand to, but you don't have to write the call to these macros. Instead, is like you had called that macros, so /. ... ./ is like /.$BeginJava ... $End Java./. Consequently, the grammars are a little bit smaller, but we obviously have some problems with some production rules such us the following one: binaryName ::= binaryIdentifier /.$NewCase./ which generates the following erroneous code: // // Rule 46: binaryName ::= binaryIdentifier // case 46: { // // Rule 46: binaryName ::= binaryIdentifier // case 46: break; } The explanation is that the /.$NewCase./ is enclosed by the $BeginJava and $EndJava macro expansion. I have been working around these problems all the day (this one and the previous one related to the $ symbol). I have also tried to find any new option to obtain the old behavior, but no luck. One solution I have found for this, is the following: - Redefine the $BeginJava and $EndJava, so that they do nothing. - Substitute $BeginJava $EndJava macro which appears in every production rule, by the correspondent $BeginAction $EndAction (note that $symbol_declarations macro is not being used by OCL). In this way, the OCLParser switch cases are well generated again. I'm not sure if this solution is liked by the team ;P. Tomorrow, I'll work on the OCLLexer/Parser/Analyzer to see if they can be easily aligned with the new lpg.runtime (v2.0.17). After that, If I manage to properly run the test-cases, I'll provide a patch with all the changes. I also want to merge the current grammars/templates with the new versions of the Templates and Maps of LPG v2.0.17 P.S: I'm in the committer nomination process for the Orbit project, so that we will able to prepare the required bundle with LPG v2.0.17 if we finally decide to go on it and we solve this bug. Robert, haven't you required to solve this concern for the IMP project? Cheers, Adolfo. Also OCLParerErrors.g etc in backtracking. I don't mind a one-off change provided the change is well defined. If BeginJava etc change, I would prefer a one-off edit that gives sensible macros probably eliminating any that do thing. It might be kind/prudent to check your well defined procedure on one of QVTr or QVTc. These may have more significant include path issues. I'm not that keen on losing the $Id$. This should at least have an LPG bug to track its resolution. Are you able to make the token types work? e.g. unary$$OCLExpressionCS ::= ... which should remove numerous casts. Hi Ed, I think I have got a first approach patch which at least make the test cases pass using lpg.runtime (LPG v2.0.17). I'll post some comments about it later, but let me write some quick replies to your comments. (In reply to comment #18) > Also OCLParerErrors.g etc in backtracking. Thank you for pointing out. I have also changed the backtracking parser stuff. > > I don't mind a one-off change provided the change is well defined. > > If BeginJava etc change, I would prefer a one-off edit that gives sensible > macros probably eliminating any that do thing. I'm not sure if I understand this. Are you suggesting finding out an alternative one?. Maybe Robert could help here, but he hasn't added any comment to the thread since it has been opened. > > It might be kind/prudent to check your well defined procedure on one of QVTr or > QVTc. These may have more significant include path issues. I'll do some checks with my QVTo grammar. I'll also write some guidelines to adopt the new way to extend OCL grammars. We must include them in the wiki so that clients are able to easily adopt these changes. > > I'm not that keen on losing the $Id$. This should at least have an LPG bug to > track its resolution. Agree, let's see if sending a bug gets more results than a forum discussion. No responses to your post in sourceforge :( > > Are you able to make the token types work? e.g. unary$$OCLExpressionCS ::= ... > which should remove numerous casts. > I haven't worked around this yet. I hope to include some advantages (including OCL API refactor) from the new grammars tomorrow, hopefully with a new patch. Cheers, Adolfo. Created attachment 144433 [details]
Patch to adop LPG v2. First approach
The attached patch contains the minimum changes I have done so that MDT-OCL is able to successfully run using LPG v2 runtime. If you want to check it you must: - download lpg.runtime (tag 2.0.17) into your workspace from CVS. - download lpg.generator.XXX (tag 2.0.17) o your workspace from CVS, if you want to generate the lexer/parser from the grammars. - take into account that API tooling errors are not fixed (actually I disabled them) since I didn't want to invest time on solve them until Bug 285354 is resolved. Apart from all I have said, I must add that I encountered some problems while treating backtracking grammars. There were weird behaviors while including parser.XXXX.gi grammars. In principle some problems where solved after setting some %Define section before the %Include sections. But I still had some problems when generating (for example an OCLKWLexer was generated instead of a BacktrackingOCLKWLexer). In the end, I'll change every %Include section by %import section. Everything properly worked like a charm. Just to remind it. This patch is a first approach so we must take into account the following: - lpg.runtime must be integrated into Orbit. with a suitable bundle name. - I want to revise and refactor the grammars. - I want to revise and refactor API to take any advantage that the new runtime offers us. P.S: I still have to work on the extending languages. I'll finally start with one of the QVT declarative languages. I'll post the experiences, tomorrow. Cheers, Adolfo. In reply to #19 >> If BeginJava etc change, I would prefer a one-off edit that gives sensible >> macros probably eliminating any that do thing. >I'm not sure if I understand this. Are you suggesting finding out an >alternative one?. Maybe Robert could help here, but he hasn't added any comment >to the thread since it has been opened. I meant that BeginJava does not appear to have the same semantics as before so it may be better to eliminate an old practice completely and replace it with a new practice to avoid a confusing mish-mash. In reply to #21 >- lpg.runtime must be integrated into Orbit. with a suitable bundle name. MDT-OCL is a widely used project. We cannot transiently impose a requirement for a non-Orbit dependency; at least not for existing functionality. This patch must therefore wait till lpg 2 is in orbit. You probably want to start a branch and maybe all the parser API changes follow that branch much more interactively till a merge in perhaps M4. ----- I'm afraid I've got too much OCL development backed up waiting for approvals to start investigating another avenue, so I will not be able to review this for a while. It's best for you to check out one of the QVTx languages first anyway. (In reply to comment #22) > In reply to #19 > > I meant that BeginJava does not appear to have the same semantics as before so > it may be better to eliminate an old practice completely and replace it with a > new practice to avoid a confusing mish-mash. Hi Ed, It's not a problem of the macro semantics. It's a problem of the generator which expects to have the BeginJava macro, so that if the macro doesn't exist in the action-block, it's implicitly introduced. I have re-read all the thread (I should have done it, earlier >.<). I tried your solution related to use the BeginCode which substitute the BeginJava one. It works but and the following warnings are thrown in the console: ...org.eclipse.ocl/src/org/eclipse/ocl/parser/EssentialOCL.gi:464:18:464:17:10733:10732: Warning: The macro "$BeginJava" is undefined. ...org.eclipse.ocl/src/org/eclipse/ocl/parser/EssentialOCL.gi:464:28:464:27:10743:10742: Warning: The macro "$EndJava" is undefined. Which prove that the generator expects the $BeginJava and $EndJava macros to automatically enclose the action-block code. Therefore, we have two options: - Remove the $BeginJava and $EndJava macros. Then, we could introduce the $BeginCode and $EndCode macros as you suggested, which would replace the original one and the would called inside the action blocks. 2 warnings would arise. - Making $BeginJava and $EndJava macros do nothing. Then, we would have to call the $BeginAction and $EndAction inside the action blocks. Both options would be practical, specially if we have the proper documentation for them. I think that those 2 warnings are painless, so I'm inclined to the first one. > > In reply to #21 > > >- lpg.runtime must be integrated into Orbit. with a suitable bundle name. > > MDT-OCL is a widely used project. We cannot transiently impose a requirement for > a non-Orbit dependency; at least not for existing functionality. This patch must > therefore wait till lpg 2 is in orbit. I agree that we have to get the bundle included in Orbit before we this bug is committed. However, I'm not solving the bundle inclusion in Orbit until we all agree about adopting LPG 2. That's the reasons about providing this partial patch, so that you can check that LPG 2 generator and runtime may be adopted without (too) pain. I have also been able to run QVTRelation junit tests using LPG 2. Again the main problems appeared while including other .gi from the grammars. For instance, QVTrParser was not able to include the EssentialOCLErrors.gi grammar file. This time replacing %Include section by %import section was not sufficient. I finally managed to solve the problem providing the include-directories as absolute paths, via arguments in the External-tool configuration: -include-directory=${workspace_loc:/org.eclipse.ocl/src/org/eclipse/ocl/parser/backtracking};${workspace_loc:/org.eclipse.ocl/src/org/eclipse/ocl/parser};${workspace_loc:/org.eclipse.ocl/src/org/eclipse/ocl/lpg}; I guess that tuning the LPG external tool configurations and adding documentation for OCL grammar extender clients should be sufficient. > You probably want to start a branch and > maybe all the parser API changes follow that branch much more interactively till > a merge in perhaps M4. I'm not very friend of branches (I didn't have good results with subversive and our local SVN server some time ago). However, I guess that branches should properly work with CVS so no problem for that. In any case, I must disagree about waiting until M4 to do the merge. As OCL developer if the grammars and infrastructure parsing is going to change we must introduce these changes as soon as possible, so that all the development efforts concerning this area is done against the new infrastructure. As OCL-extender client I would specially expect this kind of changes as soon as possible as well. P.S: I'll attach 2 patches, a new OCL infrastructure patch (just to replace BeginJava, EndJava macros by BeginCode EndCode one) a new Patch concerning QVTDeclarative so that you can check the changes concerning OCL extending languages. P.S2: The 18th on August I should officially be Orbit committer. There I would just need some +1 by the team to include LPG v2.0.17 in MDT-OCL 3.0.0 in order to start the process to have lpg v2.0.17 included in Orbit. Cheers, Adolfo. Created attachment 144559 [details]
New OCL with LPG v2 patch.
This patch substitutes the $BeginJava and $EndJava macros by $BeginCode and $EndCode.
Created attachment 144561 [details]
Patch to align an OCL Extending Language (QVTr)
The patch including needed changes to QVTr grammar files to adopt LPG v2.0.17 generator/runtime.
I don't like warnings so the approach that I happened to adopt in my abandoned attempt is not the right one. I think your solution to BeginJava must be better. Your solution to LPG invocation for QVTr is not very satisfactory; a separate special launch per project. This needs to be raised as an LPG 2 bug. If you are very close to getting LPG in Orbit I was being needlessly pessimistic about M4. I also don't like branches; we would have significant cross-merge difficulties. Hopefully Alex will +1 the 3.0.0 Bugzilla now that he's back so that I can commit that and the next part of the JUnit tests, then I can start on some more bits including this. > I don't like warnings so the approach that I happened to adopt in my abandoned > attempt is not the right one. I think your solution to BeginJava must be better. I liked yours since we can preserve the original macro behavior (which includes the $symbol_declarations macro call). So if u are unpleased with these warnings, a third option comes up which mixes both solutions: - make BeginJava/EndJava do nothing. - create BeginCode, EndCode which replaces the BeginJava/EndJava behavior. - replace BeginJava, EndJava calls by BeginCode, EndCode. > > Your solution to LPG invocation for QVTr is not very satisfactory; a separate > special launch per project. This needs to be raised as an LPG 2 bug. > I disagree on part 1. You would only need one launcher for the OCL project, actually I would include the launcher I use for OCL based on resource selection: Location: ${workspace_loc}/lpg.generator.${target.os}_${target.arch}/lpgexe/lpg-${target.os}_${target.arch} Working Directoy: ${container_loc} Arguments: -include-directory=${workspace_loc:/org.eclipse.ocl/src/org/eclipse/ocl/parser/backtracking};${workspace_loc:/org.eclipse.ocl/src/org/eclipse/ocl/parser};${workspace_loc:/org.eclipse.ocl/src/org/eclipse/ocl/lpg}; ${resource_loc} With this configuration and having the lpg.generator.XXXX project in your workspace you could execute lpgexe on every .g/.gi grammar file which needs/extends any OCL .g/.gi grammar file. > If you are very close to getting LPG in Orbit I was being needlessly pessimistic > about M4. I also don't like branches; we would have significant cross-merge > difficulties. Hopefully Alex will +1 the 3.0.0 Bugzilla now that he's back so > that I can commit that and the next part of the JUnit tests, then I can start on > some more bits including this. Well, let's see what do our remaining team mates think about this... Cheers, Adolfo Third option to BeginJava seems good. Currently I have a single launcher for LPG: select a .g file, invoke LPG launcher on current selection. This allows me to LPG the KWLexer until that works, then the Lexer and finally the Parser; and it works for all projects. I feel that, once an LPG bugzilla has been fixed, the same functionality should be available. (In reply to comment #28) > Third option to BeginJava seems good. > > Currently I have a single launcher for LPG: > > select a .g file, invoke LPG launcher on current selection. > > This allows me to LPG the KWLexer until that works, then the Lexer and finally > the Parser; and it works for all projects. That's what I refer. Exposing an external tool configuration for the OCL project and any other project which extends OCL grammar files, shouldn't worry us about the need of those arguments for the lpgexe generator. > > I feel that, once an LPG bugzilla has been fixed, the same functionality should > be available. That's what I'm not very confident. The feedback from LPG team has always been terribly poor. A help question I posted 2007-03-29, is still pending response... So I'll try to avoid be dependent of a bug fixing.... At this moment, the only thing which is really concerning me is the missing of the $Id$ in the OCLKWLexer java files. So we need some vote for this options (any extra option is welcomed) in which I include some + and -. 1) Stay on LPG v1.1 + We don't have to change nothing from the grammars, nor tricks to generate the lexer/parser. - The old fashioned runtime have some undesirable design decisions which are corrected in the new lpg.runtime. - If we tend to update the lpg runtime some day, this is the moment since we are having a major version increment. - If in the future we are adopting a new LPG version, the gap will be thiner if we stay on a LPG v2.x version. 2) Change OCLKWlexer.gi %Notice section so that we remove the $Id$. (We can miss them, or we can manually add them before commit). + We avoid option 1) and option 3) - We miss the CVS information in some generated files, unless we manually add them. 3) Remove $ token since it's not needed by OCL concrete syntax. + We avoid adopting option 1) and option 2) + We remove a warning for not using the $ token in the OCL files. - This may cause problems in an OCL-extending lexer which used $ token. Since we are in a major version increment, we are allowed to do it. I'm inclined on the third option. $ token has probably been imported from a version of the LexerTemplate. If there is no need to have this token in the grammar we could definitely remove it, avoiding the problem on the generated OCLKWLexer.java file. Ed,Alex, Laurent please add more options and/or vote on this. I have finally been promoted to Orbit committer, so If an option different to 1) wins, I'll start to work on having LPG 2.0.17 in Orbit. Cheers, Adolfo. +1 for Option 2. We can live without the CVS info until the LPG bug is fixed. Option 1 is not necessary. You have successfully overcome all the major problems. The minor ones can be tolerated. You missed commenting that the OCL/QVTc/QVTr editors use of IMP runtime uses LPG 2. Option 3 is very undesirable. $ is often used in extended contexts. We should perhaps only allow it subject to a ParserOption. (In reply to comment #30) > +1 for Option 2. > > We can live without the CVS info until the LPG bug is fixed. > > Option 1 is not necessary. You have successfully overcome all the major > problems. The minor ones can be tolerated. You missed commenting that the > OCL/QVTc/QVTr editors use of IMP runtime uses LPG 2. Good pooint. If OCL editor needs LPG 2 runtime, we have to solve the library inclusion in Orbit anyway. So I'll be on it ;) > > Option 3 is very undesirable. $ is often used in extended contexts. We should > perhaps only allow it subject to a ParserOption. > Extenders can always add their needs. However, I don't have any inconvenient of having controlled warnings or manually adding a $Id$ to the generated file. Let's see if we have more feedback. Meanwhile, I'll go on Orbit's tasks. Cheers !!! Adolfo. phew, that's some reading. I'm not that familiar with LPG. Judging by the comments and options given above, I'd choose 2). I'm not that inclined to use any CVS tag and I do not see them as being useful. The history gives better information :p. Adolfo. I've started looking at this post-3.0.0 increment. To avoid duplication of effort can you resubmit the OCL patch with a) API filters for every IToken etc change. b) @since 3.0 for new parser generation artefacts The QVTr patch probably survives since QVTd has no baseline. No rush for the @since's Since Lexprs etc have a fundamental phiolsophical change we can just declare a top level per-class API change; no need to worry about fine grained detail. Looks good. Changes look straightforward and sensible. I can regenerate the parser. Any simple way of making the lpg.exe reference easier? I'll +1 when the API filters are in place and I've checked QVTc and QVTr. (In reply to comment #34) > No rush for the @since's > > we can just declare a top level per-class API change Ed, Can I create a new API Filter per class ?. how? I have only been able to create a new API filter per error in the class. I'm on a refactoring of the templates/classes based on the new LPG API. The LPGs lexer/parser has been refactored so that now the generated Lexer doesn't extend LpgLexStream, it just contains the LpgLexStream (AbstractLexer in our case) as a field. Now doing several parsing should imply calling a method, instead of creating new instances of the lexer,parser, etc. This should increase performance in scenarios where a lot of parsing processes are needed, such as an Editor ;). I hope to get some pleasant results tomorrow. About Orbit: I should have said that I passed the official voting process, instead of saying that I'm official committer. Burocratic issues are still pending, so I can't do any commit yet. Meanwhile, I have also "pinged" IMP newsgroup to ask about their position around LPG 2.0.17 in Eclipse. I'm still waiting response. Cheers, Adolfo. No idea how to do a per-class API filter. super.getPrsStream().makeAdjunct at line 22 of LexerBasicMap.go needs to be super.getIPrsStream().makeAdjunct to avoid a deporecation warning. I have the editors working fine for QVTc, QVTr, KM3. Just some minor JUnit error message spelling to sort out. So +1, provided you commit with API filters. (In reply to comment #36) > No idea how to do a per-class API filter. > > super.getPrsStream().makeAdjunct > > at line 22 of LexerBasicMap.go needs to be > > super.getIPrsStream().makeAdjunct > > to avoid a deporecation warning. > > I have the editors working fine for QVTc, QVTr, KM3. Just some minor > JUnit error message spelling to sort out. > > So +1, provided you commit with API filters. > I have been working two days on this. I have reworked grammars, even lexer/parser infrastructure, so that an OCLParser is only a facade to parse the CST, which contains the CST-building methods and an a decoupled instance of a IPrsStream. In a similar way, the OCLLexer contains an instance of ILexStream. I have tried to recover the concept of the original recommended LPG dtParserTemplate, modifying them to adopt the current OCL . I'll also take a look at the btParserTemplate, so that I can analyze if it provides added value to current technique of macro definitions, which allow us to have the deterministic and backtracking parser version. I hope to finish the tests tomorrow, and I'll make a separate patch for the OCL Editor and the QVTr or QVTc extension (BTW I have some problems with the last CVS version of QVTd which included some comment tokens, or something like that). I'll also upload a text file which should help language extenders to migrate lexer/parser grammars and infrastructure. I have mostly tried to keep compatibility backwards. However, after this step, It could be useful analyze the current design about the different concerns in the parsing process such as BasicEnvironment, ProblemHandler, etc. It seems that some of this non-easy-to-understand design comes from some deficiencies in the old LPG runtime infraestructure and some needs about compatibility backwards, which we can reconsider in OCL 3.0.0. Cheers, Adolfo. The comments for the editors are awaiting a +1 on Bug 286724. BasicEnvironment certainly needs examination. I had to completely reimplement EcoreEnvironment and UMLEnvironment in order to share abstract functionality for an interchangeable Ecore/UML-based editor. My ICSTEnvironments with Root, Node and File variants needs to go high up. I'm not clear what problem your reorganisation solves. It just seems to make things different. Even if the organisation is a good idea we should proceed in two stages. The current patch does a good job of migrating from LPG 1 to LPG 2. Then we can see about reorganising. Hi Ed, Sorry but I have been a little bit busy this morning at work. (In reply to comment #38) > The comments for the editors are awaiting a +1 on Bug 286724. > Ok, I'll take a look to this, right now. > BasicEnvironment certainly needs examination. I had to completely reimplement > EcoreEnvironment and UMLEnvironment in order to share abstract functionality > for an interchangeable Ecore/UML-based editor. My ICSTEnvironments with Root, > Node and File variants needs to go high up. > > I'm not clear what problem your reorganisation solves. It just seems to make > things different. In practice, it should improve performance in scenarios where a repetitive parse is needed. In theory, it's just a better design style, IMO. Another point is that aligning to LPG templates style implies that future improvements of the templates evolution, should facilitate us to adopt said changes. > Even if the organisation is a good idea we should proceed in > two stages. The current patch does a good job of migrating from LPG 1 to LPG 2. > Then we can see about reorganising. > In principle, I should agree. However, since a first patch must imply migration for OCL extenders. A second patch could also imply a new migration. If we can simplify in just one step, I think it's better for MDT-OCL and OCL extenders. Besides, the patch it's nearly done, I only have to find out why are a couple of test cases failing, and align the QVTDeclarative grammars for them. I hope to finish the patch during the afternoon. After that, we can vote on our preferred patch. Two phases is undesirable yes, but they are very different. Phase 1 is a simple lexical change that we are happy with. Phase 2 needs review and will be a different kind of change for extenders. Provided we deliver both phases between M1 and M2 I see no problem at all with two phases. Although I have finished the OCL-patch, I'm afraid that you'll have to see the results next week. I want to prepare an example with the OCL-Extending language such us QVTr or QVTc, and revise all the stuff. Have a nice weekend. Cheers, Adolfo. (In reply to comment #40) > Two phases is undesirable yes, but they are very different. Phase 1 is a simple > lexical change that we are happy with. Phase 2 needs review and will be a > different kind of change for extenders. Provided we deliver both phases between > M1 and M2 I see no problem at all with two phases. > Ed, I have been working several days on this, so I want to upload the work anyway. In any case, we should even not commit to CVS the phase 1, since we have to include LPG v2 in Orbit, which is pending on my paperwork revision. Sharon is in holidays until next week, so we will have to wait a little bit. I guess that we will have some time to revise and discuss about the changes I plan to include in lexer/parser infrastructure. Cheers, Adolfo. Ok. You'll have to repeat the .g to .gi conversions when everything's ready. It's very easy so not a big problem. Created attachment 146213 [details]
provided patch
Hi Ed,
I have eventually finished this try. Sorry for the long description but it will be helpful when I create a wiki entry to describe the migration from LPG1 to LPG2.
The new versions of grammars/templates/inclusion are based on:
- The last version of the templates/includes which reside in the official CVS lpg.generator(2.0.17) project.
- how the javaParser exploits said files in the official CVS lpg.examples.java (2.0.17) project.
- how the current OCL implementation modifies the old versions of grammars/templates/includes.
That said, I'll explain the relevant changes or notes I have taken:
*** OCLKWLexer related Grammars/Templates/Includes ***
KeywordTemplateD.g to KeywordTemplateF.gi:
1. All the new template has been adopted, excepting the removal of the unnecessary %Globals section.
KWLexerMap.g to KWLexerMap.gi:
2. All the new include has been adopted. Since the %Terminal section is defined here, it will be removed from the OCLKWLexer.gi file.
OCLKWLexer.g to OCLKWLexer.gi
3. $id$ has been removed from %Notice section to avoid a stupid error in the generated OCLKWLexer.java file.
4. The %Eof and %Terminal sections have been removed since they are defined in the template and include file.
*** OCLLexer related Grammars /Templates/Includes ***
In LPG v2 the runtime API has been refactored so that a generated Lexer doesn't extend LexStream. Instead, it will contain an instance of said class, so that the lexer's API is much simpler. Therefore, we will deal with an AbstractLexer class (which will maintain the environment) and AbstractLexerStream (which will contain the logic related to ILexStream). Unfortunately since the LexStream reports the errors, and we have delegated to our BasicEnviroment's problem handler, the AbstractLexerStream must contain the BasicEnviroment as well.
NB. BasicEnviroment/ProblemHandler design should be re-analyzed.
NB. Some AbstractLexerStream methods should be analyzed to see if we can remove them. They are kept for compatibility reasons.
LexerTemplateD.g to LexerTemplateF.gi
5. The generated OCLexer will extend the AbstractLexer, which won't be the LexStream anymore. Therefore, the macro names must be accurated:
- A new macro $super_lexer_class appears which corresponds to the AbstractLexer, the super class which be extended by the generated lexer. This macro may be redefined by extenders
- the instance of the LexStream used by the template, it will defined and used by the template and the include file. This macro is not intended to be redefined by extenders
6. Some functionality which resided in the old LexerTemplateD now resides in the new LexerBasicMapF template.
7. Several constructors based on some reset methods are included in the new template:
- This resets methods are the KEY, so re-parsing will be made via this methods.
- The Reader-based constructor and reset methods has been included.
- The constructor based on no-input (only an environment as parameter) has been removed. A parse/lexer infraestructure will be always constructed on a given input.
8. Some methods related to LexStream API has been been removed, such us setInputChars.
9. Some methods related to AbstractLexer API has been kept, due to compatibility backwards, such us lexToTokens. However this method has been moved to AbstractLexer and reworked to invoke the proper lexer template's method.
LexerBasicMap.g to LexerBasicMapF.gi
10. The original LexerBasicMapF.gi defines an extension of LpgLexStream. Since we have an AbstractLexerStream a new $super_lexer_stream macro is needed (look at point 5).
11. Since now we have an internal LpgLexStream class, some constructors methods are added this include file.
OCLexer.g to OCLLexer.gi
12. It seems that the $action_class macro is not needed. Although it could be removed, I have only added a deprecated comment just for compatibilty reasons.
*** OCLParser related Grammars/Templates/Includes ***
As occurred with the Lexer, the AbstractParser doesn't extend PrsStream anymore. Instead, it will contain an instance of said class, so that the parser's API is much simpler. Same approach, explained for the lexer, has been taken for the parser.
On the other, the previous EssentialOCL.g grammar file includes the original parserTemplate code defined by LPG into the own file. The concept of parserTemplate has been rescued and the last version provided by LPg has been adopted. Actually the refactoring of parser infrastructure comes from the template itself.
dtParserTemplate.F
13. The definition section defined in Essential.OCL .gi was the original LPG dtParserTemplate.g . So, the section has been removed from there and it has been merged into the new dtParserTemplate.F
13. It has been slightly modified so that the same template may be used by a deterministic or backtrakcing parser, in the same fashion as it's currently solved via macro definitions by Essential.g grammar file.
14. Since the new LPG version scopes the block action code with a $BeginJava and $EndJava macro when generating the code, it doesn't allow us to use the useful $newCase macro. Therefore, we will make $BeginJava and $EndJava do nothing and we will create a new $BeginCode and $EndCode macros which will be used instead of $BeginJava and $EndJava.
15. Only one constructor, which receives an AbstractLexer instance is maintained. Note that the $lexer_stream_class macro has not been renamed to avoid extenders to rename this macro as well.
16. Some macro definitions (such us $parserCore) and some methods (parseTokensToCST), which were initially defined in Essential.gi have been revised and removed/reworked since they are now considered in somehow in the template and/or AbstractParser.
EssentialOCL.g to EssentialOCL.gi
17. The definition section has been removed (look at point 13).
18. %EOF and %Error section has been removed since it is defined in the template.
19. %Headers section doesn't call $parserCore anymore. The code now resides in the template (look at point 16). Besides, now includes couple of methods which avoid changing grammar action-blocks.
NB. The action-block code could be reworked in the future so that this methods can be removed. Another point is the problem that may cause extenders if we remove this methods. I think
that a discussion about what to break and not to break can be done in a different bug.
20. In section %Rules, The macro $BeginActions doesn't need to be called.
21. Every call to the $BeginJava or $EndJava macros needs to be replaced by $BeginCode and $EndCode macros (look at point 14).
22. %Trailers section is removed since it is defined in the template.
Changes to OCLParser.g
23. The main change is that now, an LPG template will be used, so that a new option will be required by the .g grammar file: %option template=dtParserTemplateF.gi
24. Some options will be removed since they are included by the template => remove table, error_maps, scopes, margin, programing_language, action-block options.
25. Now the macros which surround the action blocks code are called as $BeginCode and $EndCode (look at point 14).
*** OCL API impact ***
The main impact of the current lexer/parsing infraestructure is the following:
- Now we are not allowed to create a analyzer/parser/lexer instances without firstly providing the input (reader, array of chars, filename
or whatever).
- Secondly some used Parse/LexerStream functionality is not available as it has been so far, since the AbstractParser and AbstractLexer has been drastically simplified. One easy solution is calling getIPrsStream or getILexStream from AbstractParser and AbstractLexer respectively as I have firstly done in this patch. Another solution is invoking the current API provided by the template (such us getRhsIToken, getRhsFirstToken, etc), or even adding and using some helpful specific functionality to the AbstractParser and AbstractLexer.
*** APIFilters ***
Every method which deals with a parameter whose type belongs to the old version of LPG runtime it's considered as a new method, even though it has a previous @since tag. It could be confussing that a method which does already exist has an @since 3.0. However since API point of view makes sense, so I have increased the @since tag version to 3.0
I have done an exception with the generated "public $prs_parser_class getParser() { return dtParser; }" method defined in the dtParserTemplateF.gi file, which can't be promoted to the
AbstractParser due to that dependency on the macro. Inserting the @since tag in the template is not a good idea neither, since a extender language with a different version number would have an incorrect @since tag. So I finally decided to include an API Filter exception in that method.
Cheers,
Adolfo
Created attachment 146215 [details]
Patch which exploits new Lexer/Parser/Analyzer infrastructure
This patch have reworked the current QVTDeclarative's common infrastructure and OCLEditor to exploit the new OCL parser infrastructure.
A patch which demostrates how align extending languages is coming soon (tomorrow :) ).
Cheers,
Adolfo.
Created attachment 146307 [details]
New OCL with LPG v2 patch
This patch has been created against the last version of CVS (including bugs solved yesterday).
All the generated classes have been removed from the patch.
Created attachment 146308 [details]
Patch to align new API and extenfing languages (QVTr)
The attached patch fixes QVTr grammars/editor and OCL editor, exploiting the new parser infrastructure.
The howto migrate OCL-extending grammars:
*** Grammar changes in an OCL extending language ***
The following changes were required to the grammar files of an OCL-extending languages such as QVT Relations, so that they can be properly used by the new LPG v2.0.17 generator.
Changes to QVTrKWLexer
1. Change the old KeywordTemplateD.g used template by the new KeywordTemplateF.gi
2. Change the legacy $Include OCLKWLexer.g $End section, by the new %Import OCLKWLexer.gi %End one.
3. Change any $ symbol which was used to mark a section, by the new the new %symbol, so that, for instance, $Define...$End section turns into %Define...%End
Changes to QVTrLexer
4. Change the old LexerTemplateD.g used template by the new LexerTemplateF.gi
5. Change the legacy $Include OCLLexer.g $End section, by the new %Import OCLLexer.gi %End one.
6. Change any $ symbol which was used to mark a section, by the new the new %symbol, so that, for instance, $Define...$End section turns into %Define...%End
Changes to QVTrParser
7. Include the new OCL ParserTemplate option => add %options template=dtParserTemplateF.gi
8. Remove the unnecessary options which are already included in the template: table, error-maps, scopes, margin, programing_language, action-block
9. Change any use of the $prs_stream_class macro by the macro $super_parser_class
10. Change any $ symbol which was used to mark a section, by the new the new %symbol, so that, for instance, $Globals...$End section turns into %Define...%End
11. Change any $empty appearance by %empty.
12. Change any $BeginJava and $EndJava macro use, by $EndCode and %EndCode
Regardin the point of view of the grammars, the only different point which the migration defers from the previous one (one step) is the point 9. Regarding the point of view of API, the minor changes, which I commented yesterdary, about the new use of creating the parser/lexer and accessing their prsStream/lexStream are required.
P.S: Due to some reasons I haven't caught, the QVTrParser.g file is detected as binary file when creating the patch. So you may encounter problems when applying it, so that u will have to fix some BeginJava- to-BeginCode and EndJava-to-EndCode macros to successfully generate the QVTrParser.
P.S: The patch still needs JavaDoc comments, copyrights and probably some tuning. However it's sufficient to start discussion about it. BTW, I have received the Committer status on Orbit one hour ago, so I'll start the commented task, tomorrow :D.
Cheers,
Adolfo.
(In reply to comment #46) > Created an attachment (id=146307) [details] > New OCL with LPG v2 patch > > This patch has been created against the last version of CVS (including bugs > solved yesterday). > All the generated classes have been removed from the patch. You seem to have removed all the parser grammars too. The patch just contains some blank line insertions. Created attachment 146322 [details]
New OCL on LPG v2 patch
Oh Dear,
My apologies. I guess this is the good one.
Preliminary comments:
Change the {lpg.exe} in the launches to {lpg2.exe} so that the old LPG cannot be used accidentally.
The test_hebrew_singleQuote_135321 fails in both Ecore and UML bindings.
EssentialOcl.gi has an "Invalid" token which EssentialOcl.g does not.
Lexer.lexer() replaces lexToTokens(). lexToToken was not a verty good name,
but lexer is differently bad; not a verb, and now requires caller to getIPrsStream. Suggest chanke to lex(AbstractParser) using a verb and hiding the indirection.
Similarly change parseTokensToCST to parse() rather than parser().
AbstractOCLParser has a @SuppressWarnings("nls"). I think this should only be used in auto-generated or test modules. Real code should have per-string NLS apologies, since at one point OCL was internationalised and might well be again.
The revised architecture seems to solve a lot of the uncomfortable compatibilities I had to struggle with originally, but I think it deserves more careful thought to get it right in one go.
There really are two changes here. I do not like reviewing them together. The LPG change involves numerous trivial changes that are tedious and not that necessary to review. The architecture change involves just a few changes that are masked by the volume of trivial changes. Please prepare a first set of patches that is a no-functionality change migration to LPG2. We can commit these (perhaps to a branch to assist creation of a two stage patch) and then review the architecture revision patch.
(In reply to comment #50) > Preliminary comments: > > Change the {lpg.exe} in the launches to {lpg2.exe} so that the old LPG cannot > be used accidentally. Ok. > > The test_hebrew_singleQuote_135321 fails in both Ecore and UML bindings. That test has been "arbitrarily" failing to me, in some occasions since years ago. I guessed that comes from any character encoding reason which I don't know. I think that it's time to discover the reason ;) > > EssentialOcl.gi has an "Invalid" token which EssentialOcl.g does not. Ok, I'll take a look to this. I probably skipped something while merging. > > Lexer.lexer() replaces lexToTokens(). lexToToken was not a verty good name, > but lexer is differently bad; not a verb, and now requires caller to > getIPrsStream. Suggest chanke to lex(AbstractParser) using a verb and hiding > the indirection. > > Similarly change parseTokensToCST to parse() rather than parser(). Well, that names comes from the original template. It can obviously be changed ;) > > AbstractOCLParser has a @SuppressWarnings("nls"). I think this should only be > used in auto-generated or test modules. Real code should have per-string NLS > apologies, since at one point OCL was internationalised and might well be > again. Ok, that should be there since time ago. If it's not solved when this is applied, I'll silently add the change. > > The revised architecture seems to solve a lot of the uncomfortable > compatibilities I had to struggle with originally, but I think it deserves more > careful thought to get it right in one go. > > There really are two changes here. I do not like reviewing them together. The > LPG change involves numerous trivial changes that are tedious and not that > necessary to review. The architecture change involves just a few changes that > are masked by the volume of trivial changes. Please prepare a first set of > patches that is a no-functionality change migration to LPG2. We can commit > these (perhaps to a branch to assist creation of a two stage patch) and then > review the architecture revision patch. Although I dislike the idea, I have no option since this is matter of two. I only hope that the patch doesn't die in the branch. About including LPG into Orbit: IMP's newsgroup message about including LPG v2.0.17 has not been replied. Collaboration with IMP's team is not being as I have liked. I'll try a final private email to Robert I have discovered that an IPZilla CQ about LPG v2.0.16 has been approved, the bundle is not in Orbit though. Since LPG v2.0.16 is properly tagged in sourceforge version, I could try the patch on this lpg runtime to see if it properly works. However I would prefer including the last version. I'll ask robert if he has any intention of including the last LPG version in Eclipse. If don't receive any response we could: 1. Go straightforward on including LPG v2.0.16 bundle in Orbit. 2. Including LPG v2.0.17, which would previously involve solving legal IP revision on LPG v2.0.17. Suggestions are welcomed. Meanwhile I'll prepare the one step patch and try to see if it also works on LPG v2.0.16. Cheers, Adolfo. (In reply to comment #51) > > The test_hebrew_singleQuote_135321 fails in both Ecore and UML bindings. > > That test has been "arbitrarily" failing to me, in some occasions since years > ago. I guessed that comes from any character encoding reason which I don't > know. I think that it's time to discover the reason ;) Odd. This worked fione for me on the previous patch. I have never seen this test fail. (Spannish) character encoding may well provide an explanation. > > > > > EssentialOcl.gi has an "Invalid" token which EssentialOcl.g does not. > > Ok, I'll take a look to this. I probably skipped something while merging. Merging in such a large list of trivial changes is very hard. Much easiuer to re-edit. > > > > > Lexer.lexer() replaces lexToTokens(). lexToToken was not a verty good name, > > but lexer is differently bad; not a verb, and now requires caller to > > getIPrsStream. Suggest chanke to lex(AbstractParser) using a verb and hiding > > the indirection. > > > > Similarly change parseTokensToCST to parse() rather than parser(). > > Well, that names comes from the original template. It can obviously be changed > ;) Yes. I wasn't expecting to change this API, but your restructuring is good and establishes a much smaller more relevant interface. If we're breaking this almost internal API, I thin k we might try to get it right. > Although I dislike the idea, I have no option since this is matter of two. I > only hope that the patch doesn't die in the branch. I hope not too. I want this resolved. The LPG delay pretty much forces the branch now. I was only suggesting the branch as an easy way of creating a part1 and part2 patch. Please commit a re-edit of the original 'part1' patch to a branch so that we can concentrate on the 'part2' API improvement. When LPG is available, it should be possible to re-edit part1. part2 might survive almost unaffected by intervening progress. > > > The test_hebrew_singleQuote_135321 fails in both Ecore and UML bindings.
> >
> > That test has been "arbitrarily" failing to me, in some occasions since years
> > ago. I guessed that comes from any character encoding reason which I don't
> > know. I think that it's time to discover the reason ;)
>
> Odd. This worked fione for me on the previous patch. I have never seen this
> test fail. (Spannish) character encoding may well provide an explanation.
The problem is the Acute definition between OCLLexer.g and OCLLexer.gi. You need to configure your Eclipse environment to avoid corrupting files. This might also explain why your fully decorated name caused so many problems.
> The problem is the Acute definition between OCLLexer.g and OCLLexer.gi. I caught it this afternoon, while creating the simple patch. > You need to configure your Eclipse environment to avoid corrupting files. This > might also explain why your fully decorated name caused so many problems. Yes, I tried to avoid 'acuting' my surname. I can't figure out which is the problem. I always configure my workscape character encoding (Preferences -> General -> workspace )as UTF-8, any additional idea ?. > > Yes. I wasn't expecting to change this API, but your restructuring is good and > establishes a much smaller more relevant interface. If we're breaking this > almost internal API, I thin k we might try to get it right. > Agree, I didn't want to change too much the LPG template, so that I show that essence of the new design comes from the LPG runtime API and its templates. You may find thatm for instance, the $getIToken, $getToken, etc macros will not generate the definitive code (Actually the proper as the new macro states, a method should be used instead). I think we will have a lot of oportunities to improve the grammars and parsing infraestructure with these new changes. > > I hope not too. I want this resolved. The LPG delay pretty much forces the > branch now. I was only suggesting the branch as an easy way of creating a part1 > and part2 patch. > > Please commit a re-edit of the original 'part1' patch to a branch so that we > can concentrate on the 'part2' API improvement. > > When LPG is available, it should be possible to re-edit part1. part2 might > survive almost unaffected by intervening progress. I was working on it this afternoon, when I encountered the problem with SimpleTypeEnum. It would also be good to incorporate finished patches, such us static feature (already revised) or that relating pre, pathNameCS and such (I'll revise it tomorrow in the morning). > Yes, I tried to avoid 'acuting' my surname. I can't figure out which is the
> problem. I always configure my workscape character encoding (Preferences ->
> General -> workspace )as UTF-8, any additional idea ?.
Yes. Don't change the default presumably Unicode configuration.
UTF-8 requires that every charcter be encoded in 8 bits, so anything that isn't 7 bits gets a multi-byte recoding.
Java is Unicode.
Created attachment 146527 [details]
OCL (Experimental) om LPG v2
This patch built on experimental branch, contains the related first step changes.
Once this patch is committed, I'll try to create the second one...
Comments are welcomed
Cheers,
Adolfo.
The new patch is very incomplete trivially; no MANIFEST.mf, lpg2.exe launches, .api_filters more seriously: no AbstractAnalyzer and everything else with a changed import, no ...lpg/*.gi templates. [There is no need to pursue QVTr on this branch. I am happy that the LPG 2 change has an easy impact, that I can follow through in due course.] (In reply to comment #57) > The new patch is very incomplete > > trivially; no MANIFEST.mf, lpg2.exe launches, .api_filters > > more seriously: no AbstractAnalyzer and everything else with a changed import, > no ...lpg/*.gi templates. Sorry again, I don't really know what I did when I createf the patch Yesterday>.<. Apologies. Uploading again. > > [There is no need to pursue QVTr on this branch. I am happy that the LPG 2 > change has an easy impact, that I can follow through in due course.] Ok. I didn't worked on the QVTr changes needed to follow this OCL Branch changes. Cheers, Adolfo. Created attachment 146557 [details]
OCL (Experimental) on LPG v2
New try with patch.
Cheers,
Adolfo.
+1 minor observations: Probably want an API filter rather than @since 3.0 on AbstractParser.parseTokensToCST, setOffsets etc etc The change of IToken package doen't make a method new. reportError definitely needs sorting out. part2 will no doubt give a better structure. Indentation for xxxsym.orderedTerminalSymbols is irregular. ------------------ Have you accounted for the delay in IP approval for LPG 2.0.18? It may be possible to use parallel IP to put LPG 2.0.18 in CVS promptly, but I doubt that it can be exported in a build till IP approval is complete. (In reply to comment #59) > Created an attachment (id=146557) [details] > OCL (Experimental) on LPG v2 > > New try with patch. > > Cheers, > Adolfo. Sorry for slow response. My +1. (In reply to comment #60) > +1 > > minor observations: > > Probably want an API filter rather than @since 3.0 on > AbstractParser.parseTokensToCST, setOffsets etc etc > The change of IToken package doen't make a method new. > I said my opinion about this. From the API point of view it's a new method (You could keep both methods if you preserved the old package). However, I'm not starting a discussion about this: having an extra API filter doesn't make me feel unhappy. So, I'll commit your suggestion. > reportError definitely needs sorting out. part2 will no doubt give a better > structure. > Indentation for xxxsym.orderedTerminalSymbols is irregular. I don't understand. They are generated files :? Let me know what do you expect to incorporate it before comitting. > ------------------ > > Have you accounted for the delay in IP approval for LPG 2.0.18? > > It may be possible to use parallel IP to put LPG 2.0.18 in CVS promptly, but I > doubt that it can be exported in a build till IP approval is complete. Robert will start IP approval as soon as the release comes up. Since previous versions have already been approved, it shouldn't take too much time. Then I'll start to elaborate the Orbit bundle which we will be able to use in our project. Meanwhile this patch, and further patches will be created using LPG v2.0.17. When 2.0.18 I'll revise that all it's ok with this version. When it is included in Orbit we will be able to do the merge. Cheers, Adolfo. > > Probably want an API filter rather than @since 3.0 on > > AbstractParser.parseTokensToCST, setOffsets etc etc > > The change of IToken package doen't make a method new. > > > > I said my opinion about this. From the API point of view it's a new method (You > could keep both methods if you preserved the old package). However, I'm not > starting a discussion about this: having an extra API filter doesn't make me > feel unhappy. So, I'll commit your suggestion. > This is certainly debatable. Does a changed import change a use of the import? Maybe some API policy may help us out. Maybe the entire class is @since. I think I lean towards semantic compatibility; AbstractParser is an @since class change because the inherited PrsStream is very different; ProblemHandler is an API filter change because the IToken usage is very similar. In auto-generated code there is a more practical consideration that it may be difficult to auto-generate @since's so an API filter is much more convenient. The changed manifest and imports make the change very clear, per-method @since's are clutter and may obscure other history. > > Indentation for xxxsym.orderedTerminalSymbols is irregular. > > I don't understand. They are generated files :? Maybe there is a template tabbing error. Maybe it's a very minor LPG coding error. Not worth a great deal of effort. > Let me know what do you expect to incorporate it before comitting. All the above where minor observations not inhibiting the +1. Go for it. If the observations can be conveniently accommodated, great, if not, who really cares? Patch comitted to BRANCH (OCL_2_1_on_LPGv2_Experimental), with the following additions: - API falters added instead of increasing the @since. - AbstractLexer.reportError, missed an else which has been restored. - The counterpart .g files of the new .gi files have been removed. Cheers, Adolfo. Created attachment 154939 [details]
OCL (HEAD) on LPG v2.0.17
Comments in the next thread
The previous attachment contains the patch to make MDT-OCL be aligned to LPGv2.0.17. The patch is created against the current HEAD. Apart from the aforementioned details, there some new comments to do: - $copyright_contribution has been restored in some grammars files. - The %import section has been moved to the top of the file, so that the macros defined in the %Define sections are properly overwritten by extending grammars. - In EssentialLexer, the commented macros definition are uncommented. (They shouldn't cause any problem, after moving the import section to the top). - In OCLKWLexer, the %Define section has been removed, since the needed macros are already defined in the EssentialOCLKWLexer. The empty %Globals section also removed. - In OCLLexer, some macros have been removed from the %Define section, since they are already defined in the EssentialLexer. - The test case of the Bug 283509 made me realize that I was erroneously overriding the reportError method. Now the error reporting method used by the LexStrem is reportLexicalError, which is invoked from every error reporting method. When committing the patch, I'll remove the old .g grammars. P.S: LPgv2.0.17 will be soon in the Orbit repository. Meanwhile you can get the LPG runtime from the sourceforge CVS. Created attachment 154947 [details] Adapt to Bug 298201 commit Attached updates CSTPackage.java and .api_filters updated by Bug 298201. Attached also adds the patch for ocl.psf. Ed, Thanks for reconciling the API filters file. I think that all is (finally :) ) ready for comittment. I guess there isn't any inconvenience with the patch, is it ? I prefer to commit and close this long bug, and open a new one to deal with the refactoring to exploit LPGv2 runtime. Created attachment 155049 [details]
Better launch configs
Attached launches exploit the naming conventions used by the official 'lpg' launch to make the two OCL launches work directly from the imported LPG projects on arbitrary platforms. Spurious builds and refreshes are also fixed.
A further generic 'LPG2 selected-file' works on either OCL .g file or QVTc or QVTr .g files.
As discussed in Bug 297966, there are some test cases related to backtracking parser which are currently failing. It seems that error codes associated to parsing error is different, This provokes a change in the error message expected by the parser. On the one hand, it seems that a suspicious warning in the backtracking parser may be the cause of the change. On the other hand, QVTo and QVTd grammars which extend OCL grammars are not experimenting any problem about this, so it may be considered as a simple change about the error generated by the parser. Since I'm having holidays this week, I'll commit the patch so nobody get blocked by this and all downstream project may react this change during this week. When I'm back the 4th in January, I'll face on this again so that I can either detect why the suspicious warning is occurring and maybe the test are recovered again or change the test cases to align to the new error message. We have a month before M5 to take a decision . Cheers, Adolfo. Patch finally committed to HEAD (including removal of the old .g grammars). Ed, could you commit your contribution of the launching configurations? Alex, coud you update any building related files to make the builds use the new LPGv2.0.17 runtime ? (I would appreciate a patch to see which files are involved). When this is done I'll resolve the bug (Ed/Alex could also do it in my absence). Cheers, Adolfo. (In reply to comment #71) > Ed, could you commit your contribution of the launching configurations? Done > Alex, coud you update any building related files to make the builds use the new > LPGv2.0.17 runtime ? (I would appreciate a patch to see which files are > involved). This is difficult without an Orbit drop to build against. Any idea when an Orbit Stable build will be published? Otherwise we have to resurrect the old technology to redistribute LPG with OCL. > When this is done I'll resolve the bug (Ed/Alex could also do it in my > absence). Is there a test case demonstrating the problem? N-Build now passes, but only by using explicit HEAD tags in the map since HEAD cannot be used for LPG 2. Hopefully an Orbit S build will appear soon. (In reply to comment #72) > > When this is done I'll resolve the bug (Ed/Alex could also do it in my > > absence). > > Is there a test case demonstrating the problem? Launch org.eclipse.ocl.ecore.tests/launches/org.eclipse.ocl.ecore.tests (Standalone Backtracking).launch All tests in "Parser Backtracking Tests" fail. I'm not worried by the backtracking parser failures, which are incidentally almost the only tests that test message severity and content rather than just existence. The change of message is a nuisance but indicates that perhaps the parser is better, so I've raised Bug 298630 to review the error grammar. Testing message content is awkard so I've raised Bug 298629, which can be tackled at the same time as Bug 296991. Someone please +1 the principle of 298629. I'll submit an initial patch on Bug 298630 to make the tests pass, hopefully as both Ecore and UML tests. (In reply to comment #75) > I'm not worried by the backtracking parser failures, which are incidentally > almost the only tests that test message severity and content rather than just > existence. But when I change the tests to match the new message, the tests then fail with a null CST. This is a serious problem. The backtracking parser is not backtracking to produce a best parse. The wanted error message arise when an ERROR_TOKEN is parsed where an SimpleNameCS parse failed allowing the parse to recover. This is not happening. Perhaps it's just some trivial initialisation that needs to change to activate the recovery. As it stands, the backtracking parser is fine if there are no errors, but is unable to diagnose multiple errors in a parsed document. This renders the backtracking support useless. Adolfo; I think this is your problem unless anyone else has time. Guys, Did you file a CQ for this different Orbit dependency? Note that you've hard coded a specific version number rather than a range; that seems like a bad idea. Note too that you reexport this dependency, just like the previous dependency, and that's resulting in compile errors in org.eclipse.m2m.qvt.oml and org.eclipse.m2m.qvt.oml.cst.parser. Are you coordinating this type of change with the affected projects? Ed(willink), I would have been surprised, if that suspicious LPG related warning only produced an error messaege change. Your comments have demonstrated the real panic of the warning. As you may know, i'm away in holidays, so i'll face on this when i'm back next monday. If we (i have required some support to LPG team), can't fix this the Following weeks, I'm afraid we will have to stay in LPG v1.1 Ed(merks), Sorry for not providing links (i'm using a not userfriendly mobile phone), but CQ has been granted, even patch for QVTo has been uploaded. I guess u may find info in one of the dependent bugs. Sergey, dont commit qvto patch until this is fixed.we have time before m5. Hopefully we can complete the migration in time :) have a happy new year, Adolfo (In reply to comment #78) > Ed(merks), > > Sorry for not providing links (i'm using a not userfriendly mobile phone), but > CQ has been granted https://dev.eclipse.org/ipzilla/show_bug.cgi?id=3656 > even patch for QVTo has been uploaded. https://bugs.eclipse.org/bugs/show_bug.cgi?id=297966 > > Sergey, > dont commit qvto patch until this is fixed.we have time before m5. Hopefully > we can complete the migration in time :) If OCL is requesting QVTo to hold back, OCL should roll back. CVS HEAD is broken for anyone using both OCL and QVTo (or QVTd); ie. GMF ... > Ed(willink), > > I would have been surprised, if that suspicious LPG related warning only > produced an error messaege change. Your comments have demonstrated the real > panic of the warning. As you may know, i'm away in holidays, so i'll face on > this when i'm back next monday. If we (i have required some support to LPG > team), can't fix this the Following weeks, I'm afraid we will have to stay in > LPG v1.1 If this is not a quick fix, we must roll back. I suspect it is a simple fix which is why I have not already rolled back, but if other projects are affected we cannot allow this inconsistency and uncertainty to persist for another four and probably more days. Any other opinions? Thanks for confirming a CQ was filed. :-) Given most folks are not working the next few days, resolving the problem on Monday is probably okay. In general though, committing changes that break the stack is of course a practice to be avoided. Someone is bound to notice. :-P The PDE seems to have developed a nasty habit of inserting specific hard coded version numbers by default; please be sure to either remove that number or change it to be a range. (In reply to comment #78) > > Sergey, > dont commit qvto patch until this is fixed.we have time before m5. Hopefully > we can complete the migration in time :) Ok, QVTo is waiting till that CR be resolved (either rolled back or fixed for backtracking). Some exploring on backtracking parser
- passing '-1' instead of '100' as value of 'error_repair_count' (second argument for parseTokensToCST() call) helps with OCL backtracking tests. (look at 'public $ast_type parseTokensToCST(Monitor monitor, int error_repair_count) {..}' in EssentialOCL.gi).
According to Java Doc '-1' means that
"The number of error token recoveries is unlimited. Also, such recoveries only require one token to be parsed beyond the recovery point. (normally two tokens beyond the recovery point must be parsed) Thus, a negative max_error_count should be used when error productions are used to skip tokens."
- example template 'btParserTemplateF.gi' (template for backtracking parser that comes in lpg-generator-2.0.17 bundle) refers to 'btParser.fuzzyParse(..)'.
As for QVTo parser both solutions ('-1' as value of 'error_repair_count' argument and 'btParser.fuzzyParse()' in place of 'btParser.parse()') solve the problems with tests on error recovery behavior.
Thanks Sergey. The error productions are certainly written so that the next ; } or ) resynchronises. I think we could propagate the LPG 2 repairCount API. If repairCount == 0 use DeterministicParser.parse (current behaviour) If repairCount < 0 use BacktrackingParser.parse (unexpected current behaviour) If repairCount > 0 use BacktrackingParser.fuzzyParse (changed behaviour) or do we need to give users more control? Created attachment 155227 [details]
Fixing BacktrackingParser patch
Sergey, Thank you very much for the insight. You have saved me some debugging tasks. The previous attatchment fixes MDT-OCL counterpart. It also fixes the lpg.runtime dependency range. Ed, migration's second phase , re-introduced the use of templates (dtParseTemplateF.gi). I'll work into this to also reintroduce the btParseTemplateF.gi, so that all the customized macros to make both parser coexist are removed from the grammar files. After this, using a backtracking or deterministic parser should only imply using a different template in the options section. In principle, I have coded in the OCL facade to do the following: errorRepair == 0 => deterministic (.parse) errorRepair != 0 => backtracking (.fuzzyParse) Cheers, Adolfo. (In reply to comment #84) > Created an attachment (id=155227) [details] > Fixing BacktrackingParser patch Looks good. My +1 Clearly the LPG 2 parse() API has changed for the backtracking parser. Our existing parse(10) recovery is only partially emulated by parse(-1); it recovers on one token but doesn't limit to 10 recoveries. I cannot see the documentation on fuzzyParse() so cannot tell whether fuzzyParse() replaces parse() or not. Otherwise we perhaps need the three way test from comment 83. Either way, +1 for now since this is a big improvement. However our previous API was N>0 is N backtracking recoveries, with -ve an error. It would be nice not to change this if we are not actually changing anything, or is -1 for unlimited a new facility? Any comments on the direction for Bug 298634? Ed, Alex The attatched patch is wrong. Actually the tests only pass when error_count_repair = -1 and dtParser.parse(error_repair_count) is called. Test cases fail when dtParser.fuzzyParse(error_repair_count) is used regardless the value of the error_repair_count. So even doing what you say in comment #83 it seems that the backtracking parser won't recover if repair_count > 0. A tricky workaround to make this more consistent (make the backtracking parser always work) may be stablishing -1 as error_repair_count regardless the provided error_repair_count. I hope to find better solution during the afternoon >.<. Regards, Adolfo. (In reply to comment #88) > Ed, Alex > > The attatched patch is wrong. Actually the tests only pass when > error_count_repair = -1 and dtParser.parse(error_repair_count) is called. Test > cases fail when dtParser.fuzzyParse(error_repair_count) is used regardless the > value of the error_repair_count. So even doing what you say in comment #83 it > seems that the backtracking parser won't recover if repair_count > 0. > Sad news. > A tricky workaround to make this more consistent (make the backtracking parser > always work) may be stablishing -1 as error_repair_count regardless the > provided error_repair_count. I hope to find better solution during the > afternoon >.<. It would be nice to have a better workaround - the QVTO code completion parser sets error_repair_count to a relatively small number since in case of numerous errors and infinite error_repair_count the parser's response may be very slow. > > Regards, > Adolfo. If necessary we can presumably count the number of error messages we have generated and throw a sufficiently brutal exception to terminate the parse. For QVTd I use a derived ProblemHandler with error and warning count limits set by default at 100. (In reply to comment #88) Hi Adolfo, Analysis of BacktrackingParser.parse(..) method reveals the following: public Object parse(int max_error_count) throws BadParseException { return parseEntry(0, max_error_count); } public Object parseEntry(int marker_kind, int max_error_count) throws BadParseException { ... if (max_error_count > 0 && tokStream instanceof IPrsStream) max_error_count = 0; ... } And AbstractOCLParser is an instance of IPrsStream. So calling parse(..) with any 'max_error_count > 0' results in 'max_error_count = 0' which means that "no Error token recoveries occur". I believe that this is guard on using parse(..) method with limits on the number of Error token recoveries. And is indirect reference that fuzzyParse(..) should be used for such purpose. So I'm agree with comment 83. > > The attatched patch is wrong. Actually the tests only pass when > error_count_repair = -1 and dtParser.parse(error_repair_count) is called. Test > cases fail when dtParser.fuzzyParse(error_repair_count) is used regardless the > value of the error_repair_count. So even doing what you say in comment #83 it > seems that the backtracking parser won't recover if repair_count > 0. > > A tricky workaround to make this more consistent (make the backtracking parser > always work) may be stablishing -1 as error_repair_count regardless the > provided error_repair_count. I hope to find better solution during the > afternoon >.<. > Hi All,
I have got some more conclusions:
- The LPGv1 (and probably LPGv2) backtracking parser should parse a file without throwing any kind of exception (BadParseException), if an error token is detected, the backtracking parser should recover and use one of the error productions to go on the parsing process.
- I believe that fuzzyParse should be used in LPGv2.0.17, since the template designed to be used by the grammars (btParserTemplateF.gi) invokes the said method.
- fuzzyParse is not working using any error_repait_count value. The problem if that after detecting an error token, it can't recover: A RecoveryParser is created, which fails the recovering (invoking fixError in line 102) and then a BadParseException is thrown.
- parse error (is partially working). It seems to be a deprecated method or the previous version of the backtracking parse method. It's only working with an unlimited error_repair_count because of a modification of this method (I don't why this was introduced):
if (max_error_count > 0 && tokStream instanceof IPrsStream)
max_error_count = 0;
So no way to establish an specific max_error_count, while using the parse version.
The point here, is why the fuzzyParser is failing when trying to recover. Probably the supression of the suspicious warning may help. I hope it's not a LPG Runtime error itself, somebody should previously have noticed it.I'll add phillippe to the bugzilla to see if he can give us a hand. I
I think that we could temporally make max error_repair_count be -1 (the painless solution) so unlimited count is used when using the backtracking version. I prefer having a "slow" backtracking parser than a non-backtracking one. This should make downstream projects to adopt the change while we solve the specific issue (probably in a separate bugzilla).
What do you think ?.
Cheers,
Adolfo.
Sergey, Mid-air collision. Some comments: - I believe that parse is not related with fuzzyParse. They are independent implementations. - Are you sure QVTo counterpart works with fuzzyParse method ?. I had to use parse method + error_repair_count = -1 to make the QVTo tests work. Cheers, Adolfo. > The point here, is why the fuzzyParser is failing when trying to recover.
> Probably the supression of the suspicious warning may help.
Sorry, I forgot to paste the warning:
OCLParserErrors.gi:35:60:35:70:971:981: Warning: The terminal symbol "ERROR_TOKEN" was not imported from OCLBacktrackingLexer.gi
(In reply to comment #93) Hi Adolfo, > > - I believe that parse is not related with fuzzyParse. They are independent > implementations. Yes, I think the same. And fuzzyParse() is the new implementation for the backtracking with limited number of Error token recoveries. > - Are you sure QVTo counterpart works with fuzzyParse method ?. I had to use > parse method + error_repair_count = -1 to make the QVTo tests work. Yes, I'm sure :) For the QVTo fuzzyPars() method diagnoses/recovers any number of errors. From the unit tests point of view it works just the same as parse(-1). About BadParseException. Looks like it throws when top level rule for the error input is not correctly defined. I mean that it's grammar problem not the parser. For the QVTo quite complicated script with arbitrary number of errors is parsed by the fuzzyParse() method without BadParseException. Regards, Sergey (In reply to comment #95) Hi Adolfo, > > > - Are you sure QVTo counterpart works with fuzzyParse method ?. I had to use > > parse method + error_repair_count = -1 to make the QVTo tests work. > > Yes, I'm sure :) > For the QVTo fuzzyPars() method diagnoses/recovers any number of errors. From > the unit tests point of view it works just the same as parse(-1). > Looks like I understand why you're not successful with QVTo unit tests. We have our own template for the parser that is located in miscellaneous.gi (suppresses template from EssentialOCL.gi). Btw, I've done some modifications to the grammar comparing to the patch you submitted. I'll attach new patch to the bug 297966 tomorrow. > > Looks like I understand why you're not successful with QVTo unit tests. We have > our own template for the parser that is located in miscellaneous.gi (suppresses > template from EssentialOCL.gi). Yes, but I simple changed the call from in the QVToParser to invoke fuzzyParse and test failed, they didn't using parse one (with the -1 as error_repair_count). I should have done something wrong... > > Btw, I've done some modifications to the grammar comparing to the patch you > submitted. I'll attach new patch to the bug 297966 tomorrow. You mean QVTo patch ?. I'm just creating OCL counterpart to suport Comment 85 (filing in a little bit). BTW, how are we dealing with the contribution?. I'm not sure if it needs a new CQ for it >.<. In any case I guess we can discuss that in the other bug. Cheers. Adolfo. Created attachment 155269 [details] Backtracking fixing patch The attachment provides problem's fix (including modified generated files), using the policy suggested by Comment 85. It contains some nasty macro's use, which may be cured when reinserting the LPG templates. Cheers, Adolfo. +1 for the interim fix. This should allow Bug 298634 to go in too. Patch committed to HEAD. I guess that QVTo may go ahead with its patch. Cheers, Adolfo. AbstractParser.setLexStream and AbstractParser.resetLexStream need to be @Deprecated to propagate the deprecation of super.resetLexStream(lexStream). Hi OCL team, Could you please produce Integration build with the actual LGP 2 dependency. TIA, QVTo team ;) (In reply to comment #102) > Hi OCL team, > > Could you please produce Integration build with the actual LGP 2 dependency. > > TIA, > QVTo team ;) Hi QVTo team ;) The I-build is available at http://www.eclipse.org/modeling/mdt/downloads/index.php?project=ocl&showAll=0&showMax=5 Cheers, - Alex. (In reply to comment #103) > > Hi QVTo team ;) > > The I-build is available at > http://www.eclipse.org/modeling/mdt/downloads/index.php?project=ocl&showAll=0&showMax=5 > > Cheers, > - Alex. Hi Alex, Thanks a lot! Cheers, Sergey I have revised some dependent bugs. It seems that only Bug 298542 should be fixed in order to resolve this one. It hasn't resolved yet, but a decision has already been taken, which should not affect MDT-OCL. Therefore, I'm resolving this bug as fixed. Closing |
The Eclipse IMP project provides useful automation for editors, especially those using LPG. This could be very useful for OCL-based languiages such as QVT. I therefore started modifying org.eclipse.ocl to use LPG 2.x rather than LPG 1.x so that IMP could be more naturally exploited. Eventually I backed out nearly all my changes since they are not necessary and create more problems than they solve. A summary of these activities is provided below ----------------------------------------------------------------- IMP provides a builder so *.g files are automatically built. Seems like a good idea, but only when there are no errors. If there are problems, the OCLParsersym.java is set to bad content and it's difficult to know which grammar is bad. I disabled the builder quite rapidly. IMP provides an LPG editor, which works well for syntax colouring and outlines but has some strange ideas about where the errors are. IMP (the Ganymede synchronised release) has a bad bug whereby any edit of a file with an error hogs CPU. A bit tedious when LPG files have spurious errors. LPG 2 uses lpg.runtime rather than lpg.lpgjavaruntime so any change introduces a binary incompatibility. lpg.runtime is not in Orbit. IMP exploits LPG 2 parsers and lexers through very small ILexer and IParser interfaces. It would be good for these to be directly supported by OCLParser and OCLLexer, however IMP is an incubation project, so introducing an MDT-OCL dependency on IMP is undesirable. LPG 2 deprecates $ so e.g. $empty needs changing to %Empty. Unfortunately preventing '$' being converted to '' defeated me in the KWLexerMap, so I had to change the escape to # rather than $. This mostly worked, but for unexplained reasons the NewCase macro for EssentialOCL.gi got a BeginJava/EndJava wrtapped around it. Changing all BeginJava/EndJava to BeginCode/EndCode cured the problem. Similar macros not in an include did not have this problem. LPG 2 seems to have an ordering problem with include paths. I had to change the %Import section to provide an explicit path. This may be because LPG 2 has the IMP paths built-in. LPG2 appears to introduce bison-style production type suffixes such as unary$$OCLExpressionCS ::= ... This didn't work for me with either $$ or ##. ------------------------------------------------------------------ IMP also supports non-LPG grammars, so I investigated use of the existing LPG 1 grammars as non-LPG IMP grammars. Very little problem, just need to clone IMP's SimpleLPGParseController. Consequently, I recommend that MDT OCL delay migrating to LPG 2 until LPG 2 and IMP are more mature. The only significant changes I found worth making to OCL were: ------------- Change CSTNode.startOffset:EInt to CSTNode.startToken:IToken Change CSTNode.endOffset:EInt to CSTNode.endToken:IToken Add CSTNode.getStartOffset() for compatibility Add CSTNode.getEndOffset() for compatibility Modify all setOffsets() to use setXxxToken rather than setXxxOffset. Only the dotArrowExpCS ::= NUMERIC_OPERATION '(' argumentsCSopt ')' causes trouble through use of setXxxOffset in a non-IToken fashion. Perhaps this partitioned IToken usage deserves revisiting anyway. ------------- Add to AbstractLexer public abstract int [] getKeywordKinds(); so that IMP can access the list of tokens for colouring.