Closed eclipse-ocl-bot closed 1 month ago
eclipse-ocl-bot
commented
1 month ago By Christian Damus on Jul 30, 2008 15:33
The API already has an OCLSyntaxHelper that, for better or worse, is intended to support the syntax-completion capability of editors. To what extent do you think that addresses IMP's requirement? Could we enhance it, rather than the Environment API?
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Jul 31, 2008 03:04
Yes. OCLSyntaxHelper looks very promising. Everything's private so cannot be enhanced. But it's internal, so evolving it to an extensible facility poses no API problems.
OCL operates in a single context so has a Root Environment for that context. This is subject to 'push'/'pop' changes for nested contexts.
QVT has a much more complex context, supported by a hierarchy of XxxEnvironment classes that define the Xxx-specific lookup semantics. [Using Java as an analogy, a lookup could occur within a JavaAnonymousClassEnvironment within a JavaFunctionEnvironment, within a JavaClassEnvironment within a JavaClassEnvironment within a JavaPackageEnvironment within a JavaPackageEnvironment; the JavaAnonymousClassEnvironment imposes the special 'final' semantics]
For simple batch parsing, the environments can be created, used, and garbage collected.
For completion proposals, the environments must be cached or recreated or duplicated. Recreation is slow. Duplication is anathema. So it may be necessary for CSTNodes to directly or indirectly reference their Environment to allow reevaluations. Possibly a change to CSTNode. Possibly a change of the OCL 'push'/'pop' policy to create a nested Environment that avoids modifying the parent.
Once the environments are available, OCLSyntaxHelper seems like a good starting point for further work. I'll study it in more detail; IMP likes Visitors even if I and EMF don't.
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Oct 15, 2008 02:37
I now have this working.
The OCLSyntaxHelper approach seems to amount to an independent slightly intuitive duplicate parser. This may be appropriate for an expression tree, but doesn't extend well to more general structures.
I have instead opted to re-use the existing CST and AST models and AST meta-model to support completion proposals in a largely language independent fashion:
Given some text location, the narrowest covering CST node can be identified. CSTNode.getAst() then gives the corresponding AST node. A cross-reference of all uses of the AST node gives all EStructuralFeatures that use that node and consequently an EClassifier constraint on proposals. All possible AST nodes can then be filtered according to the EClassifier constraint, the required name prefix constraint, and optionally a scoping/visibility constraint.
In the above, only the scoping/visibility constraint is language-specific. Sometimes TRUE is a good solution.
This approach makes use of CSTNode.getAst() and the AST, so it is important that a structurally equivalent AST is always built and that CSTNode.getAST() is accurate. Ensuring that the AST is structurally equivalent requires unresolved value nodes to stub out leaves; createDummyInvalidLiteralExp() does this at present within OCL. It may need strengthening to be more precise.
CSTNode.getAst() has a number of omissions and a few errors at present; the ast property is very new and currently set only by initASTMapping(). Most SimpleNameCS nodes have no associated AST node. FeatureCallExpCS.SimpleNameCS\ incorrectly maps to the PropertyCallExp rather than the EStructuralFeature.
I can workaround the getAst() problems by derived tweaks, however it would seem better to try to get it right. Many of the fixes are simple one-liners, but a couple require more extensive working around a missing CSTNode argument in an AST resolving function. e.g. I have to fix-up getCollectionType() in a derivation of every method that invokes it.
Are you happy to receive a patch comprising the one-liners and new functions such as getCollectionTypeCS with the missing argument that then invokes getCollectionType for API compatibility. Or does the protected nature of these methods obviate the need for compatibility?
eclipse-ocl-bot
commented
1 month ago By Christian Damus on Oct 15, 2008 08:56
(In reply to comment #3)
I now have this working.
The OCLSyntaxHelper approach seems to amount to an independent slightly intuitive duplicate parser. This may be appropriate for an expression tree, but doesn't extend well to more general structures.
Yes, this originated back in the ANTLR days and wasn't updated for the LPG CST-based parser implementation until relatively recently. There are still a number of holes, most notably related to features of implicit iterator variables.
Are you happy to receive a patch comprising the one-liners and new functions such as getCollectionTypeCS with the missing argument that then invokes
Absolutely. The parser must ensure that every CST that has a corresponding AST node is linked to it. A patch will be most appreciated.
getCollectionType for API compatibility. Or does the protected nature of these methods obviate the need for compatibility?
A compatible solution will have to account for the protected methods, because they are API for subclasses. The delegation of new protected API to the old API should provide the compatibility with clients that have overridden the old API but (obviously) haven't overridden the new API.
Thanks!
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Oct 17, 2008 12:27
There are far fewer subrotines that need fixing; beauce getCollectionType was wrong I assumed that they were all the same; no the other are ok.
The most awkward problem is terminating the AST reference from an "iterate" OperationCallExpCS.SimpleNameCS, because there is no corresponding 'meta-function' in OCLStdLibrary.
a) The simplest solution would be to add "iterate" and "iterator" to collection iterator operations. This is not wrong, but not entirely right either.
b) At present I have added a third class of operations; 'keyword' operations.\ This ripples rather more than I would like.
c) Perhaps these AST references could be just left as null.
d) Perhaps OCLStdLibrary acquires two special purpose singleton operations.
Which solution do you prefer?
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Oct 24, 2008 11:00
Created attachment 116056 (attachment deleted)\
Fix for 242236 and 251303
The attched provuides numerous one line fixes to make CSTNode.setAST work and improve the accuracy of some initASTMappings. Unfortunately not all AST to CST mappings are symmetric, so BasicEnvironment2 introduces an assmetric variant of initASTMapping without breaking the signature API. The current implementation of initASTMapping keeps the last of a series of mappings. This is very difficult to accommodate within the current re-use structure of simpleNameCS. The code now keep the first of a series of mappings, which ensures that any gratuitous and often unhelpful re-assignments in outer productions leave the setting established closest to the AST-CST equivalence undisturbed.
I managed to clean up the code very slightly in a few places, but preservation of API makes significant improvements in clarity difficult.
[The extra annotation on the packages in TypeResolverImpl.java is not a necessary part of the patch; it just makes my checking for full AST to CST and CST to AST mapping easier; annotated objects do not need a CST origin]
eclipse-ocl-bot
commented
1 month ago By Christian Damus on Nov 02, 2008 10:23
(In reply to comment #5)\
The most awkward problem is terminating the AST reference from an "iterate" OperationCallExpCS.SimpleNameCS, because there is no corresponding 'meta-function' in OCLStdLibrary.
Yes, the whole iterator question is quite awkward. The OCL Abstract Syntax doesn't actually identify iterators as features of any kind, not operations nor anything else. There are real holes in the specification, here. The 2.1 RTF has an issue to address the publication of the OCL models, and the MDT OCL 2.0 release will then be able to adopt these along with aligning to the language changes. Until then, I'd rather not poke at the stdlib if it can be helped.
c) Perhaps these AST references could be just left as null.
This would be my preference.
(In reply to comment #6)
Created an attachment (id=116056) [details] Fix for 242236 and 251303
I like the intent of the code to fill in missing AST/CST traceability holes. However, there are a number of problems that need to be fixed in this patch:
Some questions:
why is it necessary to create dummy packages? This is the first\ instance, I think, of creating a dummy model element to\ represent something that wasn't found in the user model. This\ is very different from dummy sub-expressions to stub out\ ill-formed OCL. It looks particularly red-flaggish when the\ default implementation just returns null. Do we need to create\ dummy classes, operations, and properties, too? If not, then\ why not?
how can CST/AST mappings be asymmetric? It doesn't make sense\ to me that traceability could be inconsistent in this way. Can you\ explain why this is useful, perhaps with a specific example?
[The extra annotation on the packages in TypeResolverImpl.java is not a necessary part of the patch; it just makes my checking for full AST to CST and CST to AST mapping easier; annotated objects do not need a CST origin]
If it isn't strictly necessary, I'd rather not dedicate the OCL parser's annotation source to this purpose at this time. We only have the one.
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Nov 03, 2008 03:22
The most awkward problem is terminating the AST reference from an "iterate"\ OperationCallExpCS.SimpleNameCS, because there is no corresponding\ 'meta-function' in OCLStdLibrary.\
Yes, the whole iterator question is quite awkward. The OCL Abstract Syntax doesn't actually identify iterators as features of any kind, not operations nor anything else. There are real holes in the specification, here. The 2.1 RTF has an issue to address the publication of the OCL models, and the MDT OCL 2.0 release will then be able to adopt these along with aligning to the language changes. Until then, I'd rather not poke at the stdlib if it can be helped.
Yes. That's why I introduced resolveIteratorOperation. My derived implementation creates an iteratorsClass with the required operations.\ This avoided any stdlib changes, which were clunky and strangely stopped working.
\
c) Perhaps these AST references could be just left as null.\
This would be my preference.
This is the behaviour of the default resolveIteratorOperation.
(In reply to comment #6)
Created an attachment (id=116056) --> (https://bugs.eclipse.org/bugs/attachment.cgi?id=116056) [details] Fix for 242236 and 251303\
I like the intent of the code to fill in missing AST/CST traceability holes. However, there are a number of problems that need to be fixed in this patch:
- 8 JUnit test failures in the org.eclipse.ocl.uml.tests suite
- API Tools problems. Please use API Tools for OCL development
- MDT OCL uses braces on all loops and if-statement clauses. Some legacy code doesn't conform; I fix it up incrementally when I make changes in the vicinity
- missing //$NON-NLS markers
- missing @deprecated doc comments
- incomplete doc comments
I've just read the API wiki, but I get nowhere. There are no baselines\ for me to select.
I tried the unit tests a while ago, but they didn't work. I just tried them again; I get 1470 out of 1521 failures. The problem is that they assume that certain environment variables are set. Perhaps you could default them, eliminate them or provide a launch configuration that works direct from CVS.
java.lang.NullPointerException\ at org.eclipse.ocl.ecore.tests.AbstractTestSuite.initFruitPackage(AbstractTestSuite.java:687)\ at org.eclipse.ocl.ecore.tests.AbstractTestSuite.setUp(AbstractTestSuite.java:186)\ at junit.framework.TestCase.runBare(TestCase.java:128)\ at junit.framework.TestResult$1.protect(TestResult.java:106)
Some questions:
- why is it necessary to create dummy packages? This is the first instance, I think, of creating a dummy model element to represent something that wasn't found in the user model. This is very different from dummy sub-expressions to stub out ill-formed OCL. It looks particularly red-flaggish when the default implementation just returns null. Do we need to create dummy classes, operations, and properties, too? If not, then why not?
I hate the dummy package too. The problem is that a complete OCL file\ can bypass the package declaration and go straight to package contents.\ However the CST has a synthetic packageDecl that points to no package.\ [The dummy package is solely for the OCL file editor; QVT doesn't need it.]
In my derived AbstractQVTAnalyzer, I do create further dummy elements. Originally I had a single Unresolved model from which I could pick a class stub or whatever, however that lost AST to CST traceability, so now I create synthetic error elements to try to ensure that the CST always translates to a maximally plausible AST.
[Preview of upcoming Bugzilla. I found that the current parser is dreadful at offering completions for partial code. Any serious syntax error leads to a null CST. The problem is that the DeterministicParser does no error recovery. Converting to the BacktrackingParser is trivial, but bound to trip an overpedantic API test. The grammar can then be expanded with additional ERROR_TOKEN productions so that parsing continues up to some repair_count limit and the ERROR_TOKEN actions provide relevant messages. From a rational API perspective this is no change with massive benefit in a file-based context; for OCL in an expression-based context multi-error reporting is less important.]
So yes, attempting to always produce a plausible CST and then always produce a plausible AST requires additional error/dummy nodes.
When you have a spare moment, you might care to glance at org.eclipse.qvt.declarative.parser, which has two additional layers, one generic hierachical CST behavioour and another of QVT-specific behaviour. You consider how much at least of the CST layer you care to migrate into the generic LPG/CST/OCL support.
- how can CST/AST mappings be asymmetric? It doesn't make sense to me that traceability could be inconsistent in this way. Can you explain why this is useful, perhaps with a specific example?
Consider SimpleNameCS, which should really map to a String in the AST. This would be almost useless, since a String has not context.
Instead a defining SimpleNamesCS such as the name of an invariant constraint, maps to the AST Constraint which also maps to the InvCS.\ Asymmetry 2 CST to 1 AST, 1 AST to 1 CST.
Alternatively a referencing SimpleNameCS, such as a type name maps to the resolved type, which does not normally map back to the CST since a type may be referenced many many times. Asymmetry CST to 1 AST, 1 AST to 0 CST. If the type is a synthetic collection type, it is nice to associate the type with the first declaration, so CST to 1 AST, 1 AST to 1 CST.
Ditto PathNameCS, which incidentally would be much better as a List of SimpleNameCS rather than a list of String; too much an API ripple for a not too difficult workaround.\ ---\ Consider syntactic sugar: now a single CST node may trigger multiple AST nodes; implicit collect, iterate variables, self variable, ...
[The extra annotation on the packages in TypeResolverImpl.java is not a\ necessary part of the patch; it just makes my checking for full AST to CST and\ CST to AST mapping easier; annotated objects do not need a CST origin]\
If it isn't strictly necessary, I'd rather not dedicate the OCL parser's annotation source to this purpose at this time. We only have the one.
This annotation is already present on EClassifier, EOperation and EAttribute. e.g see EcoreEnvironment.defineAttribute.\ The omission from EPackage seemed like an oversight that cannot be rectified with trivial reliable code. The omission from EParameter is easily rectified by a parent check.\
eclipse-ocl-bot
commented
1 month ago By Christian Damus on Nov 05, 2008 13:36
(In reply to comment #8)
I've just read the API wiki, but I get nowhere. There are no baselines for me to select.
As described in the API Tools guide, you create API Baselines by pointing the tool at an Eclipse installation (or a PDE target) that is at the required version line-up. Myself, I have two development environments:
I keep my Galileo environment current with Eclipse, EMF, and UML2 by updating frequently. In this workbench, I created an API Tools baseline simply by pointing it at my Ganymede installation.
I tried the unit tests a while ago, but they didn't work. I just tried them again; I get 1470 out of 1521 failures. The problem is that they assume that certain environment variables are set. Perhaps you could default them, eliminate them or provide a launch configuration that works direct from CVS.
1521? That suggests that you're trying to run the org.eclipse.ocl.standalone.tests. That project already has a launch config:
/org.eclipse.ocl.standalone.tests/launches/MDT OCL Tests - Stand-Alone Mode.launch
You should see it in the External Tools menu already. If you don't, you can make Eclipse aware of it by invoking the "Run As" menu on the *.launch file. One of the UML tests fails with a security exception in this mode, on an attempt to shutdown an executor. The next time I'm in that area, I'll see how to fix this test.
The alternative, of course, is to run the org.eclipse.ocl.uml.tests.AllTests as a JUnit plug-in test. That is quite sufficient for most purposes.
Some questions:
- why is it necessary to create dummy packages? This is the first instance, I think, of creating a dummy model element to represent something that wasn't found in the user model. This is very different from dummy sub-expressions to stub out ill-formed OCL. It looks particularly red-flaggish when the default implementation just returns null. Do we need to create dummy classes, operations, and properties, too? If not, then why not?
I hate the dummy package too. The problem is that a complete OCL file can bypass the package declaration and go straight to package contents. However the CST has a synthetic packageDecl that points to no package. [The dummy package is solely for the OCL file editor; QVT doesn't need it.]
Ah. Perhaps, then, the CST needs to introduce that top-level CS node that you suggested in another bug, and it can contain a list of context declarations, which are a mix of package and classifier contexts. That additionally requires a common supertype for the PackageDeclarationCS and the ClassifierContextDeclCS, but eliminates the need for a dummy package.
What do think of that?
When you have a spare moment, you might care to glance at org.eclipse.qvt.declarative.parser, which has two additional layers, one generic hierachical CST behavioour and another of QVT-specific behaviour. You consider how much at least of the CST layer you care to migrate into the generic LPG/CST/OCL support.
Do you mean the ErrorNode and IdentifiedCS? I'm afraid I don't see any hierarchical structure, there, nor much behaviour ...
- how can CST/AST mappings be asymmetric? It doesn't make sense to me that traceability could be inconsistent in this way. Can you explain why this is useful, perhaps with a specific example?
Consider SimpleNameCS, which should really map to a String in the AST. This would be almost useless, since a String has not context.
Instead a defining SimpleNamesCS such as the name of an invariant constraint, maps to the AST Constraint which also maps to the InvCS. Asymmetry 2 CST to 1 AST, 1 AST to 1 CST.
But the InvCS includes the SimpleNameCS, so why should the SimpleNameCS map to the Constraint, also? It can map to the String as its AST and indirectly to the Constraint via the containing InvCS. The traceability is already there ...
Alternatively a referencing SimpleNameCS, such as a type name maps to the resolved type, which does not normally map back to the CST since a type may be referenced many many times. Asymmetry CST to 1 AST, 1 AST to 0 CST. If the type is a synthetic collection type, it is nice to associate the type with the first declaration, so CST to 1 AST, 1 AST to 1 CST.
Is the AST-to-CST mapping in this case actually useful or interesting? The same AST referenced from different CSTs can have vastly different meanings, according to the containing expression context. This multiplicity would seem to occur only with ASTs that are user-model elements, not in the expressions that actually are tree nodes in the AST. So, I think there is always some expression that is 1-to-1 mapped to a CST that provides the valuable traceability.
In the example of the SimpleNameCS referencing a class in the user model, the class may be referenced as the type of a let-variable declaration in one case and the referredClassifier of a TypeExp in an oclIsKindOf() call in another. These are very different contexts, in which in the latter case has traceability from the TypeExp to the CST and the former case has traceability of the Variable to the CST.
In any case, EMF's inverse-referencing capability gives us these reverse mappings.
The AbstractBasicEnvironment currently assumes that an AST maps to at most one CST, and in OCL at least is used only to produce error messages. I think that, perhaps, the recently-introduced AST references from the CST can obsolete the mapping that the environment maintains?
Ditto PathNameCS, which incidentally would be much better as a List of SimpleNameCS rather than a list of String; too much an API ripple for a not too difficult workaround.
This was a rather literal implementation of the OCL spec, I think. Not that this was necessarily a good idea, considering how far behind the AST and UML 2.x it is ...
Consider syntactic sugar: now a single CST node may trigger multiple AST nodes; implicit collect, iterate variables, self variable, ...
Yes, but how would these traces actually be used? Say I have an implicit collect like self.ePackage.eClassifiers.name. The collect-iterator expression in its entirety should trace to the ".name" CST, but what point is there in having any of the internal structure of the iterator also trace to ".name"?
I'm beginning to think that an overhaul of the parsing infrastructure in support of rich text editors for OCL documents makes a good theme for the 2.0 release, and that it doesn't really fit into 1.3.
[The extra annotation on the packages in TypeResolverImpl.java is not a necessary part of the patch; it just makes my checking for full AST to CST and CST to AST mapping easier; annotated objects do not need a CST origin]
If it isn't strictly necessary, I'd rather not dedicate the OCL parser's annotation source to this purpose at this time. We only have the one.
This annotation is already present on EClassifier, EOperation and EAttribute. e.g see EcoreEnvironment.defineAttribute.
Yes, but the purpose of the annotation here is to store the definition constraint-expression in the attribute. The package has no such definition with which to annotate it, and the only packages in the TypeResolver's resource are just there to organize demand-created types. They have no relationship with either the CST or the AST.
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Nov 06, 2008 02:38
Quick response to most of #9.
Ah! 'Add' fooled me. I misread as 'create' baseline. Looks interesting but I get stuck with a single org.eclipse.ocl 'should be 2.0' that persists even when I unroll back to CVS. There seems no clue as to what the breakage is; just that it was there before my change, which I think is API-compatible, since BasicEnivironment2 is a checked optional enhancement not a mandatory imposition.
No I get 1521 org.eclipse.ocl.ecore.tests that cannot be run.
Yes, a TopLevelCS could fix dummy package. I don't have the UML tooling to update the CT to your satisfaction though.
I was referring to the CSTFileEnvironment that supervises a CSTRootEnvironment that may have a hierarchy of CSTChildEnvironment. The potential hierarchy is realised in the QVTc and QVTr plugins and also in Adolfo's QVTo.
Ah. I missed the constrainmt contents. But then the new annotation is a place holder for a set of constraints that happens to be empty.
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Nov 11, 2008 12:48
Created attachment 117572 (attachment deleted)\
Misc fixes
However, there are a number of problems that need to be fixed in this patch:
- 8 JUnit test failures in the org.eclipse.ocl.uml.tests suite
Seemed more like 20. Problem was that null rather than OclVoid was in use as the previously unchecked operation return type. Fixed.
- API Tools problems. Please use API Tools for OCL development
Finally got this going. API tools are even flakier than usual about plugin/resource usage, often reporting spurious no changes or a useless all-changed.
7 missing @since's added for this patch.\ API filter added for all the changed parser symbols (not this patch).
- MDT OCL uses braces on all loops and if-statement clauses. Some legacy code doesn't conform; I fix it up incrementally when I make changes in the vicinity
You have your style sheet; I don't. I think it's easiest for you to do Control Shift F as you review.
- missing //$NON-NLS markers
8 added. If OCL uses stronger checking than Eclipse default, perhaps OCL should provide its stronger .settings.
- missing @deprecated doc comments
1 found.
- incomplete doc comments
1 superfluous @param found; is that what you mean?\ at least 20 remain in existing code
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Nov 12, 2008 02:52
Created attachment 117633 (attachment deleted)\
Corrected patch
Please ignore the previous attachment; I must have used the wrong workspace.
[API tooling is causing me a lot of trouble: Bug #254988]
eclipse-ocl-bot
commented
1 month ago By Christian Damus on Nov 12, 2008 21:33
(In reply to comment #10)\
JUnit tests.
No I get 1521 org.eclipse.ocl.ecore.tests that cannot be run.
Odd. "Run As -> JUnit Plug-in Test" in the context menu of the AllTests class should just work.
TopLevelCS, dummy package
Yes, a TopLevelCS could fix dummy package. I don't have the UML tooling to update the CT to your satisfaction though.
I just use the generated UML tree editor. No special tools. There are one or two other bugs that need something like the TopLevelCS to fix them, so I'll take a stab at it.
Hierarchical struicture
I was referring to the CSTFileEnvironment that supervises a CSTRootEnvironment that may have a hierarchy of CSTChildEnvironment. The potential hierarchy is realised in the QVTc and QVTr plugins and also in Adolfo's QVTo.
Ah, I see. I guess the "Environment" in the name is why I didn't recognize these.
There seems to be a lot in these environments that OCL doesn't need ... and dependencies on AbstractFileHandle, XMIResource, and others that would have to be factored out. I don't think I can do much, here.
(In reply to comment #11)\
However, there are a number of problems that need to be fixed in this patch:
- 8 JUnit test failures in the org.eclipse.ocl.uml.tests suite
Seemed more like 20. Problem was that null rather than OclVoid was in use as the previously unchecked operation return type. Fixed.
Oh, cool.
- MDT OCL uses braces on all loops and if-statement clauses. Some legacy code doesn't conform; I fix it up incrementally when I make changes in the vicinity
You have your style sheet; I don't. I think it's easiest for you to do Control Shift F as you review.
I do Ctrl+Shift+F. The problem is that JDT's formatter doesn't add missing braces. There's probably more that it doesn't do, besides, but this stood out.
I use the UML2 project's formatter settings, because they happen to look just right. :-)\ I have updated the OCL project settings to include this formatter profile and shared it as OCLCodeFormatting.xml in the org.eclipse.ocl.releng project.
- missing //$NON-NLS markers
8 added. If OCL uses stronger checking than Eclipse default, perhaps OCL should provide its stronger .settings.
Good idea. I've done that.
- missing @deprecated doc comments
1 found.
- incomplete doc comments
1 superfluous @param found; is that what you mean? at least 20 remain in existing code
I don't recall how many of these problems there were. I try to update existing code as whenever I happen to need to work on it, and try to ensure a standard in new code. I don't have the time to sweep through all of the existing codebase.
eclipse-ocl-bot
commented
1 month ago By Christian Damus on Nov 12, 2008 22:38
(In reply to comment #12)
Thanks, it certainly applies much more cleanly (though the .apifilters changes appear to be superfluous).
However, as I mentioned in comment #9, I'm not convinced of the value of these asymmetric CST/AST mappings for OCL. I would like to understand how it would be useful to the OCL component before I adopt it, because otherwise it seems that the environment adapter strategy currently employed by QVTr makes it relatively easy to implement this at that level.
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Nov 13, 2008 15:08
Thanks, it certainly applies much more cleanly (though the .apifilters changes appear to be superfluous).
It depends how the API support is feeling. I find it very intermittent. One workspace works nicely. Another fails totally. It seems that @noreference doesn't work so every 'public' Parser constant mmay eventually need to be disclaimed.\
However, as I mentioned in comment #9, I'm not convinced of the value of these asymmetric CST/AST mappings for OCL. I would like to understand how it would be useful to the OCL component before I adopt it, because otherwise it seems that the environment adapter strategy currently employed by QVTr makes it relatively easy to implement this at that level.
\ Finally completing my reply to comment #9.
A common problem to be resolved by an editor is convert a pixel position into useful information appropriate to the pixel position.
StyledText converts the pixel position to a character offset.\ A tree search of the CST finds the CST node with the shortest token range that covers the offset.\ Interrogation of the CST node yields an AST object that provides the information.
Most positions are either covered by an identifier (SimpleNameCS or PathNameCS) or keyword. Keywords are omitted from the CST, so a keyword is covered by the CST construct caused by the keyword.
It is identifiers that are most interesting, since a CST construct may have many identifiers with many different roles. One may define e.g. an attribute name while another may define an attribute type, the latter name being semantically resolved to reference some appropriate AST object. So a SimpleNameCS may be a definition of its parent's name or a parameterisation of its parent with externally resolved names.
It is convenient, though not essential to have both the defining SimpleNameCS and the parent CST point at the defined AST, with the AST mapping back to the parent CST. Omission of the SimpleNameCS to AST mapping could be easily deduced from its parent since there is only one defining name.
It is much more important for the referencing SimpleNameCS to point at the referenced AST. The reverse mapping is generally missing since an AST can map to at most one CST and a type may be referenced in many places. If this asymmetric mapping is omitted, every time a referencing SimpleNameCS must be converted to an AST, it is necessary to determine the role of the SimpleNameCS with respect to its parent and then locate the corresponding role in the parent's corresponding AST. This correspondence is likely to be dependent on specific language semantics, so the resolution to AST cannot be language neutral.
The Analyzer is inevitably very language specific, so it is convenient for the analyzer to establish language-specific CST-AST relationships so that subsequent usage can be substantially language neutral.
The above was motivated by simple hover text response.
It also works for refactoring. All SimpleNameCS that map to the same\ AST are subject to change when renaming any of the SimpleNameCS nodes.\ Language-specific semantics are only needed to validate name spellings and check that the revised name does not create hierarchical name resolution conflicts.
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Dec 04, 2008 02:56
Is there anything I can do to help incorporate this patch?
It's getting quite inconvenient maintaining QVT Declarative for Quentin and Adolfo as a CVS'd patch on a slightly vintage OCL CVS Tag.
It would be really good to get this in for M4.
eclipse-ocl-bot
commented
1 month ago By Quentin GLINEUR on Dec 04, 2008 04:26
I confirm!\ If there is anything I can do also, do not hesitate to tell.
Thanks in advance.
(In reply to comment #16)
Is there anything I can do to help incorporate this patch?
It's getting quite inconvenient maintaining QVT Declarative for Quentin and Adolfo as a CVS'd patch on a slightly vintage OCL CVS Tag.
It would be really good to get this in for M4.
eclipse-ocl-bot
commented
1 month ago By Christian Damus on Dec 21, 2008 12:43
Created attachment 121028 Revision of patch #117633
Attached a modification of patch #117633, with the following changes:
I would like to commit these changes, but before I can, I have some questions:
Thanks.
:notepad_spiral: bug242236-redux.patch
eclipse-ocl-bot
commented
1 month ago By Adolfo Sanchez-Barbudo Herrera on Dec 22, 2008 07:45
Hi all,
Just an implementation detail to preserve backward compatibility: In spite of a client usually extends the abstract environments provided by MDT OCL project, clients could implement BasicEnvironment (included in OCL 1.2) by itself, and they could try to adapt to the said interface.
Therefore the cached environments must be BasicEnvironment. The hashmap and the OCL.getBasicEnvironment must be changed to deal with BasicEnvironment instead of BasicEnvironment2.
I'm not uploading a new patch just for that (maybe a test case which may use a dummy environment which implement BasicEnvironment and doesn't implement BasicEnvironment2 ?). Christian, take into account this before committing to CVS
Cheers,\ Adolfo.
eclipse-ocl-bot
commented
1 month ago By Christian Damus on Dec 22, 2008 09:11
(In reply to comment #19)
Therefore the cached environments must be BasicEnvironment. The hashmap and the OCL.getBasicEnvironment must be changed to deal with BasicEnvironment instead of BasicEnvironment2.
I think this is already covered ... The OCLUtil does provide a BasicEnvironment2 as an adapter for the BasicEnvironment type, so clients will still get the latter if they ask for it.
Or am I misunderstanding your comment?
Please, do provide a JUnit test if it shows a problem!
eclipse-ocl-bot
commented
1 month ago By Adolfo Sanchez-Barbudo Herrera on Dec 22, 2008 09:32
(In reply to comment #20)
(In reply to comment #19)
Therefore the cached environments must be BasicEnvironment. The hashmap and the OCL.getBasicEnvironment must be changed to deal with BasicEnvironment instead of BasicEnvironment2.
I think this is already covered ... The OCLUtil does provide a BasicEnvironment2 as an adapter for the BasicEnvironment type, so clients will still get the latter if they ask for it.
Or am I misunderstanding your comment?
No, you aren't >.<
Indeed, an AbstracBasicEnvironment is created for both. I don't know where my mind went off.
The patch looks nice for me... I need internal refactoring for performance, due to I was keeping mappings in my environments, but exploiting the new implementation is my task.
Good work.
eclipse-ocl-bot
commented
1 month ago By Ed Willink on Dec 22, 2008 13:49
Looks very good.
I have a couple of further NPE traps, which are probably better dealt with by a follow-up Bugzilla, rather than increasing this one's complexity.
The intent of the message argument to createDummyInvalidType was to allow a recipient of an OclVoid to examine or regenerate the original error message. I do not override this method, so the extra argument could be removed; however it is perhaps a useful flexibility.
assertion of the self variable is a dropping from my experiment to see if there were any existing clients; I think you will find that the caller always creates first.
I can easily work around not providing the Ecore TypeResolver change; it's only used in missing setAst detection code. That said it would be more consistent if it was present.
operationCS defines an operation, so the operation is the target of the operationCS and simpleNameCS. operationCallExpCS references an operation so the operation is the target of just the simpleNameCS; an OperationCallExp is the target of the operationCallExpCS.
Message, State and AssociationClass are not part of EssentialOCL, so I just made an attempt to do something sensible. Without any OCL source using them, I satisfied myself with not breaking your JUnit tests. I may well have got these a bit wrong. If you have some interesting OCL text using these concepts, I can put it through my editor and tidy off these loose ends. Either way there are no existing customers, so the patch can go in and then be refined.
I'll update the BackTrackingParser patch when this is in CVS.
eclipse-ocl-bot
commented
1 month ago By Christian Damus on Dec 30, 2008 06:51
The modified patch is committed to HEAD (1.3 branch).
Thanks for the contribution!
eclipse-ocl-bot
commented
1 month ago By Ed Willink on May 27, 2011 02:48
Closing after over 18 months in resolved state.
| --- | --- | | Bugzilla Link | 242236 | | Status | CLOSED FIXED | | Importance | P3 enhancement | | Reported | Jul 28, 2008 09:46 EDT | | Modified | May 27, 2011 02:48 EDT | | Version | 1.2.0 | | Reporter | Ed Willink |
Description
IMP-generated editors support completion proposals, so OCL-based editors would benefit from some enhanced lookup support.
The current Environment API was designed to support 'resolve-a-single-definition' and was slightly revised to support 'resolve-a-single-or-ambiguous-definition'.
Completions require a more substantial, possibly filtered set of alternative returns.
I guess an additional optional/adaptable Environment interface is appropriate.
Any suggestions or related functionality that such an interface might pick up?