Open eclipse-ocl-bot opened 5 days ago
eclipse-ocl-bot
commented
5 days ago By Adolfo Sanchez-Barbudo Herrera on Sep 03, 2016 12:45
Repository-Branches:\ ocl-master (commit 1894bbae4d78d59763946645703df867f08138b8) \ qvtd-asanchez/500803\ cs2as-asanchez/500803
Repro:
Run: org.xtext.example.companies.build\src\org\xtext\example\companies\build\mwe\GenerateCS2ASTx.mwe2
[NB cs2as URI: https://github.com/adolfosbh/cs2as]
Regards,\ Adolfo.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 03, 2016 12:46
There was bad recursion when implementations of Boolean::and were used, so it was disabled. I'm surprised you are managing to use it.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 03, 2016 13:04
(In reply to Adolfo Sanchez-Barbudo Herrera from comment #1)
- Repository-Branches: ocl-master (commit 1894bbae4d78d59763946645703df867f08138b8) qvtd-asanchez/500803 cs2as-asanchez/500803
No need for cs2as-asanchez/500803. Example1_CG/2_CG/2V2_CG give
java.lang.StackOverflowError\ at org.eclipse.ocl.pivot.internal.OperationCallExpImpl.eGet(OperationCallExpImpl.java:269)\ at org.eclipse.emf.ecore.impl.BasicEObjectImpl.eGet(BasicEObjectImpl.java:1011)\ at org.eclipse.emf.ecore.impl.BasicEObjectImpl.eGet(BasicEObjectImpl.java:1003)\ at org.eclipse.emf.ecore.impl.BasicEObjectImpl.eGet(BasicEObjectImpl.java:998)\ at org.eclipse.emf.ecore.util.EcoreUtil$Copier.copyAttribute(EcoreUtil.java:626)\ at org.eclipse.emf.ecore.util.EcoreUtil$Copier.copy(EcoreUtil.java:483)\ at org.eclipse.emf.ecore.util.EcoreUtil$Copier.copyContainment(EcoreUtil.java:599)\ at org.eclipse.emf.ecore.util.EcoreUtil$Copier.copy(EcoreUtil.java:490)\ at org.eclipse.emf.ecore.util.EcoreUtil$Copier.copyContainment(EcoreUtil.java:599)\ at org.eclipse.emf.ecore.util.EcoreUtil$Copier.copy(EcoreUtil.java:490)\ at org.eclipse.emf.ecore.util.EcoreUtil$Copier.copyContainment(EcoreUtil.java:599)\ at org.eclipse.emf.ecore.util.EcoreUtil$Copier.copy(EcoreUtil.java:490)\ at org.eclipse.qvtd.codegen.qvti.analyzer.QVTiAS2CGVisitor.createCopy(QVTiAS2CGVisitor.java:522)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.inlineOperationCall(AS2CGVisitor.java:1001)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.generateOperationCallExp(AS2CGVisitor.java:620)\ at org.eclipse.qvtd.codegen.qvti.analyzer.QVTiAS2CGVisitor.generateOperationCallExp(QVTiAS2CGVisitor.java:588)\ at org.eclipse.qvtd.cs2as.compiler.internal.CS2ASJavaCompilerImpl$CS2ASAS2CGVisitor.generateOperationCallExp(CS2ASJavaCompilerImpl.java:314)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.visitOperationCallExp(AS2CGVisitor.java:1370)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.visitOperationCallExp(AS2CGVisitor.java:1)\ at org.eclipse.ocl.pivot.internal.OperationCallExpImpl.accept(OperationCallExpImpl.java:528)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.doVisit(AS2CGVisitor.java:375)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.visitLetExp(AS2CGVisitor.java:1282)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.visitLetExp(AS2CGVisitor.java:1)\ at org.eclipse.ocl.pivot.internal.LetExpImpl.accept(LetExpImpl.java:506)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.doVisit(AS2CGVisitor.java:375)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.visitLetExp(AS2CGVisitor.java:1282)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.visitLetExp(AS2CGVisitor.java:1)\ at org.eclipse.ocl.pivot.internal.LetExpImpl.accept(LetExpImpl.java:506)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.doVisit(AS2CGVisitor.java:375)\ at org.eclipse.ocl.examples.codegen.analyzer.AS2CGVisitor.inlineOperationCall(AS2CGVisitor.java:1023)
eclipse-ocl-bot
commented
5 days ago By Adolfo Sanchez-Barbudo Herrera on Sep 03, 2016 13:06
(In reply to comment #3)
(In reply to Adolfo Sanchez-Barbudo Herrera from comment #1)
- Repository-Branches: ocl-master (commit 1894bbae4d78d59763946645703df867f08138b8) qvtd-asanchez/500803 cs2as-asanchez/500803
No need for cs2as-asanchez/500803. Example1_CG/2_CG/2V2_CG give
Oh yes, of course :)
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 03, 2016 13:22
The problem has been there all along.
It was 'fixed' by an isLookupOp fudge in the CS2AS generateOperationCallExp override. The underlying generateOperationCallExp has inlining suppressed.
As the code stands, it might be necessary to do some content analysis to detect recursion and disable inlining accordingly.
Once we consider operation evaluation caching, operation inlining becomes a bit suspect. Recursion is almost guaranteed to be inhibited by an evaluation re-use check.
I don't see any point looking at this until we have the operation evaluation cache working,.
eclipse-ocl-bot
commented
5 days ago By Adolfo Sanchez-Barbudo Herrera on Sep 03, 2016 13:44
(In reply to comment #5)
Once we consider operation evaluation caching, operation inlining becomes a bit suspect. Recursion is almost guaranteed to be inhibited by an evaluation re-use check.
Indeed. I think that the thought of in-lining+caching concern came to my mind while ago. Well, let's see if at least, the scalability experiment show clear benefits of caching operations.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 06, 2016 13:11
Avoiding inlining when calling a non-final operation avoids the infinite recursion and is a simple heuristic for where a cache might help.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 25, 2016 12:13
ocl-asanchez/500803 and qvtd-asanchez/500803 exploit the caches but get compilation errors on 4 of the OCL2QVTi TestCases.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 25, 2016 12:14
It's a calling convention error. The call returns an OCL CollectionValue when an EMF Collection was expected, and is then converted to an OCL CollectionValue.
The cached operations are calling convention blind. Object[] => Object, so it works for any calling convention. However something should have adjusted to what's in use.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 25, 2016 12:16
The cached operations are calling convention blind, but the CG calling of them is far from blind. My pragmatic reuse of CGNativeOperation which is unboxed was a mistake. OCL Branch ewillink/500803 adds a CGCachedOperation and CGCachedOperationCallExp that are boxed as required for OCL Expressions. No more compilation errors. The examples run but four give minor comparison errors that I have seen in the past as a result of ignored dependencies. Might be the 'same' problem again, but more likely one of your lookups has returned null, so I'll leave you to take a first look.
eclipse-ocl-bot
commented
5 days ago By Adolfo Sanchez-Barbudo Herrera on Sep 25, 2016 13:07
Thanks.
Note that this is just a different implementation solution of "OCL-correct" lookup functionality. So I don't expect bad OCL code nor dependencies.
This is definitely doing more than what I was observing last time I was debugging this. Good. I presume that the current problem is related to the dynamic dispatch in this differently generated code. I recall dynamic dispatch was causing you troubles. Note that contributions to the lookup environment is done by ovearloading code. If the overloaded code is executed instead, no contributions are done, hence no successful lookups...
I'm gonna debug further to see if the problem is related to this rationale.
Regards,\ Adolfo.
eclipse-ocl-bot
commented
5 days ago By Adolfo Sanchez-Barbudo Herrera on Sep 25, 2016 13:27
Hi Ed,
Indeed. The lookup execution starts with an invokation of "INST_OclElement_unqualified_env_B", the default (empty) environment computation. And in the set of cached operation, I see no corresponding code for:\ \ context A1\ def : _unqualified_env_B(child : ocl::OclElement) : lookup::LookupEnvironment =\ if ownsB->includes(child)\ then parentEnv_B().nestedEnv() .addElements(ownsB->select(x | self.ownsB->indexOf(x) < self.ownsB->indexOf(child))) \ else parentEnv_B()\ endif\ \ NB. Although with the same goal of this bugzilla, the inlining issue seems to be solved (?). At least with the new 50083 branches the transformation is executing sensible code (even though not all the sensible/required code is in/taken into account).
Regards,\ Adolfo.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 25, 2016 13:41
(In reply to Adolfo Sanchez-Barbudo Herrera from comment #12)
I see no corresponding code for:
context A1 def : _unqualified_env_B(child : ocl::OclElement) : lookup::LookupEnvironment =
This is presumably two problems.
a) all derived operations should be generated too.
b) the appropriate derived operation should be dynamically dispatched.
I had hoped that just adding
protected class CACHE_OclElement__lookupA1 extends CACHE_Visitable__lookupA1
to
protected class CACHE_Visitable__lookupA1 extends AbstractEvaluationOperation
would dispatch automatically, but I don't think we have the right 'this' for such a simple solution.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 25, 2016 14:00
The cache works correctly for re-use, it is just the first use that needs to dispatch dynamically, so we just need one generated class method for the root type method, where the basicEvaluate() performs an instanceof search to invoke the appropriate basicEvaluateXXX for type XXX. A smarter table lookup is probably not worthwhile since there will only be a few types which an efficient binary tree search should dispatch quickly.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 26, 2016 13:37
(In reply to Ed Willink from comment #14)
A smarter table lookup is probably not worthwhile since there will only be a few types which an efficient binary tree search should dispatch quickly.
But building a tree needs an ability to reason about Java class inheritance and for some obscure GenModel cases we only have the spelling of a future class. The smarter lazily built JavaClass-to-'operation' can be inherited and easily generated and initialized.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 27, 2016 04:41
OCL Branch ewillink/500803 now generates error free code for all the OCL2QVTi test cases with virtual dispatch and all the operations. However the same four tests fail for the same probably lack of dependency / evaluation error reasons.
Attempting to debug a simpler start-from-the-tx-class test case highlights some model registration issues that have been overlooked for too long.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 27, 2016 05:24
(In reply to Ed Willink from comment #16)
Attempting to debug a simpler start-from-the-tx-class test case highlights some model registration issues that have been overlooked for too long.
QVTd branch ewillink/500803 fixes some registrations, only two failures. Some polymorphic functionality must now be working. Registration pain still to be investigated.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 27, 2016 07:23
(In reply to Ed Willink from comment #16)
Attempting to debug a simpler start-from-the-tx-class test case highlights some model registration issues that have been overlooked for too long.
Your use of platform:/plugin/org.eclipse.ocl.pivot/model-gen/oclstdlib.ecore rather than platform:/resource/org.eclipse.ocl.pivot/model-gen/oclstdlib.ecore causes unresolved proxy issues. platform:/resource falls back to platform:/plugin but not vice-versa.
TargetMM1.genmodel and EnvExample1.genmodel are mutually usedGenPackages and both have an oclstdlib.genmodel. This seems to hit a GenModel bug whereby each thinks that the other will provide the OCLstdlibPackage initialization and neither does requiring a manual declaration to repair it. Eliminating the spurious TargetMM1.genmodel usage of oclstdlib.genmodel solves the problem.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 27, 2016 10:12
(In reply to Ed Willink from comment #16)
Attempting to debug a simpler start-from-the-tx-class test case highlights some model registration issues that have been overlooked for too long.
Missing genmodels give a GenModelException for every genmodel-less feature within CG2Java, but they are all absorbed until an obscure UnsupportedOperationException appears. Accumulating them in CodeGenerator.problems and appending the problems to any final exception is much more useful, albeit verbose.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 28, 2016 07:10
(In reply to Ed Willink from comment #18)
This seems to hit a GenModel bug whereby each thinks that the other will provide the OCLstdlibPackage initialization
See Bug 502405.
eclipse-ocl-bot
commented
5 days ago By Ed Willink on Sep 28, 2016 07:28
After fixing ancilliary issues such as Bug 502405, 502378, 502352 suddenly all the tests work.
OCL ewillink/500803 a83292b50e77ce0ce0ab29c1470ad1b32c36afe0\ QVTd ewillink/500803 0a306c71e1ade1a69c92a75fec43130734931e07
TODO: Regenerating "QVTd Test Models" gives errors on the visitors. To what extent are these obsolete, and to what extent are there more evaluation dispatch cases to debug?
TODO: tidy up the commits
eclipse-ocl-bot
commented
5 days ago By Adolfo Sanchez-Barbudo Herrera on Sep 28, 2016 12:15
Hi Ed,
It seems that you have been dealing with nightmare/headache issues. Thanks.
I was expecting to finally discover if the visitors can be deprecated and replaced by the cached operations, in order to decide if we can get rid of visitors or not. On the other hand, I noticed yesterday/two days ago that some cached operation functionality is already in master, and the lookup infrastructure generation is currently broken. So:
Tomorrow, I'll try to adopt all the new code, and make the companies example use it, to see if we finally get some encouraging conclusions.
NB. Perhaps, LookupVisitors + some cache-based re-design could be even faster than the new cached operations approach.
| --- | --- | | Bugzilla Link | 500803 | | Status | NEW | | Importance | P3 normal | | Reported | Sep 03, 2016 12:30 EDT | | Modified | Sep 28, 2016 12:15 EDT | | Reporter | Adolfo Sanchez-Barbudo Herrera |
Description
While doing some experiments to replace the CGed code for a bunch of lookup OCL operations rather than the Lookup Visitors, I've spotted a bug that causes an infiite loop when in-lining the CGed code for an OperationCallExp.
As soon as I get the bug id, I'll upload a proper repro.