-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inconsistent multiple inheritance renaming #108
Comments
By Ed Willink on Feb 08, 2021 03:46 A first attempt to trim the example to a smaller OCL-free repro failed. There may be an additional complexity, perhaps dependent on GenAnnotation settings. |
By Ed Willink on Feb 08, 2021 04:13 Created attachment 285475 (In reply to Ed Willink from comment #1)
Seems a bit temperamental, but eventually the attached generates a bad Duck3.Impl. :compression: Bug571005.zip |
By Ed Willink on Feb 08, 2021 04:24 (In reply to Ed Willink from comment #2)
Seems to be necessary to Reload the GenModel after changing any GenAnnotation otherwise user is in a mysterious stale state. (After Reload there a heap of warnings wrt the UML metamodel).
seems to come and go as
is changed. DISCARD ok, PROCESS bad. |
By Deniz Eren on Feb 08, 2021 05:18 There are still some side-effects when this option is done too. In the example I last attached for example most errors go away except for: |
By Ed Willink on Feb 08, 2021 05:25 Unfortunately the documentation on the options is thin. I would expect that the default options would all work. Non default should only be used by users who understand how a particular option may give some beneficial corruption/optimization. Unfortunately many options such as opposite-role-names have come in as fixes to legacy deficiencies. |
By Deniz Eren on Feb 08, 2021 18:24 Hi Ed, I tried the default settings and played with this duplicate feature also but error still remain, but is much better than before. At least now I can manually edit the Java code and fix it to work after generation. It may not be ideal but I can already appreciate how difficult it will be to resolve all general cases of this issue without significant refactor. Thank you for your efforts. |
By Ed Willink on Feb 09, 2021 00:47 Surely the consistently first / not-first workaround is easy enough to arrange? |
By Deniz Eren on Feb 09, 2021 02:03 Hi Ed, |
By Ed Willink on Feb 09, 2021 05:22 If you can't predict it then just declare something like HasName as the first transitive superclass. You may need to Reload the genmodel to see the effect of any change. |
By Deniz Eren on Feb 09, 2021 05:46 Yes that’s a good idea |
By Deniz Eren on Feb 09, 2021 05:48 Ed, otherwise these fixes are all good - the only concern now is that will these fixes all make it to the official releases of Eclipse? |
By Deniz Eren on Feb 09, 2021 08:35 Ed, even with this common base element; thus all multi-inheritance elements only redefine the base_Element of the root common stereotype, I get a lot of cast issue in derived property generated code such as: final /@NonInvalid/ Comment base_Comment_0 = aDocumentSection_0.getBase_Element(); |
By Deniz Eren on Feb 09, 2021 08:35 Of course this can be fixed with a simple cast; but there are 90+ instances... |
By Ed Willink on Feb 09, 2021 13:07 All the OCL fixes should be in M3. I've made good progress on a much improved allInstances/implicitOpposites cache that will more than offset the cost of expanding the exten from the Resource to the ResourceSet and thereby considering the whole of all UML metamodels. Not sure when UnlimitedNatural will be fixed. This particular bug is a UML2 bug and since it depends on some slightly strange user models that have a reasonable workaround. I don't anticipate a rush of enthusiasm amongst the maintainers. Just possibly the OCL support could barf when it sees inconsistent multiple inheritance ordering. |
By Ed Willink on Feb 09, 2021 13:12 (In reply to Deniz Eren from comment #12)
A line of code with /@NonInvalid/ in comes from OCL2Java and so a missing cast is an OCL CG bug. Please raise a Bugzilla with a repro; probably just need the *.profile.uml. |
By Deniz Eren on Feb 09, 2021 19:32 Hi Ed, I only uploaded the file "resources/profile/ValidationProblem.profile.uml" as you've asked. If you find you require the rest of the project please let me know and I will zip the whole thing up and send also. But the only difference in genmodel end is the: \
to DISCARD.
|
By Deniz Eren on Feb 09, 2021 20:02 Hi Ed, |
By Deniz Eren on Feb 09, 2021 20:07 So if these 2 new bugs are fixed, then together with single common Stereotype which the whole profile ultimately inherits and thus all Stereotypes redefine that central respective property of base_Element or otherwise. Also together with the duplicate feature option being put to Discard in the genmodel, there will be zero Java errors reported (once these 2 new bugs are fixed). |
By Ed Willink on Feb 23, 2021 05:50 (In reply to Ed Willink from comment #3)
No the problem is comparitively 'simple'. Duck3 extends both CanFly and then CanSwim but because something else has CanFly as a second extension, base_NamedElement in CanFlyImpl was renamed to base_CanFly_NamedElement. Duck3 just needs to ensure that for a same-named redefinition it uses the redefined GenFeature where it should find the disambiguated name base_CanFly_NamedElement rather than the redefining base_NamedElement. (Bug 571407 identified this need for same-named/new-named redefinition distinction when sythesizing Jav for OCL code.) |
By Deniz Eren on Feb 23, 2021 06:35 Yes actually this duplicate feature configuration set to discard does fix the reference to the correct dis-ambiguous naming issue. In some complex model I'm getting failures of selectByKind and oclIsKindOf for types, even after the duplicate feature workaround - does this sounds like it could be related to this same issue? Would it help if I try to replicate it in a simple example? |
By Ed Willink on Feb 23, 2021 06:58 (In reply to Deniz Eren from comment #20)
Definitely not a UML issue. Please raise an OCL Bugzilla with repro. |
By Deniz Eren on Feb 23, 2021 07:24 Let's see if I can replicate it with simple test profile.. |
By Deniz Eren on Feb 24, 2021 00:54 The issue regarding selectByKind isn't easy to replicate with small model. For example take the simple case getNormalVariable() function that calls getVariable() and selects all the elements that are of Kind CMakeNormalVariable which inherits CMakeVariable: /**
I've printed the size of the List "variable" and then the size of the list "ECORE_selectByKind". The results are interesting; randomly at around 50%-50% the following results are seens when the model that uses this static profile gets shutdown and reloaded. Sometimes prints: Other times prints: This type of different resulting outcomes usually result from race-conditions or memory leaks. So I don't know how to easily provide a model to replicate this behaviour. I also tried adding this to various parts of the code with no particular outcome: Do you recomment any experiments I can do in the Java code to see if we can narrow down what is causing this randomly occurring failure? I remember you memtioned there was some memory leaks you found? Let me know if there is anything I can do and re-test to narrow down the issue? |
By Deniz Eren on Feb 24, 2021 01:10 P.S. with this new version: Build #258 (22 Feb 2021, 11:51:05) |
By Ed Willink on Feb 24, 2021 04:22 Please do not clutter bugs with extraneous issues. It irritates the irrelevant developers and the clutter gets overlooked by the relevant developers. A leak is a brand new problem. selectByKind is an OCL issue. If it reproduces as a compilation failure, that is pretty easy to fix. Send me the model by email if you are concerned about [posting a proprietary repro. |
By Deniz Eren on Feb 24, 2021 04:52 My apologies. I will try to see if the inconsistent issue is a problem from my model or try to narrow it down, then will create a new bug ticket. Definitely isn't a compile issue with selectByKind, but runtime behaviour in Papyrus - what product should I raise the bug for in that case? |
By Ed Willink on Feb 26, 2021 05:01 (In reply to Ed Willink from comment #19)
Bug 571494 identified that pragmatic use of name/originalName is unnecessary. UML2Ecore already provides a 'redefines' EAnnotation.references that enables any non-EClass-contained EStructuralFeature to locate the correct EStructuralFeature for which a GenFeature provides the appropriate Java spellings. |
By Ed Willink on Feb 26, 2021 13:44 Debugging. It appears that a GenFeature has been created for a "duplicates" Ecore feature which "redefines" the Ecore feature that has been renamed to disambiguate. There seems to be a simple principle. EClass-contained features are reified in Ecore. EAnnotation-contained features are not and no GenGeature should ever be created for such a feature. |
By Ed Willink on Feb 26, 2021 16:09 Debugging a bit further I find GenClassImpl.getImplementedGenFeatures seems to be where a bad GenFeature comes from, but it uses union/superset/duplicates that I don't understand ... However I see everything is based on getName() which I have learned to be a really bad way to code since names can be ambiguous and renamed. I suspect the code should be using originalName unless of course EObjects are not possible. |
| --- | --- |
| Bugzilla Link | 571005 |
| Status | NEW |
| Importance | P3 normal |
| Reported | Feb 08, 2021 03:20 EDT |
| Modified | Feb 26, 2021 16:09 EDT |
| Blocks | 571494 |
| See also | 570891, 571407, 571074 |
| Reporter | Ed Willink |
Description
The second attachment to Bug 570891 evolves a troublesome static profile example in which the renamed multiple inheritance detected that OCL was not going back to the original name.
The evolved example variously has CanFly as a first inheritance that is not renamed and as a second inheritance that is. Consequently the names accessed by Duck3 use the wrong naming.
file:/E:/Development/Chital/Workspace/_OCL_UsageTests__testBug570891a_uml/test-src/Bug570891a/validationproblem/impl/Duck3Impl.java
58:21 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
58:50 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
59:81 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
60:25 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
61:29 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
63:173 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
66:24 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
74:9 method does not override or implement a method from a supertype
76:24 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
86:53 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
87:17 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
89:153 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
98:24 cannot find symbol
symbol: variable base_NamedElement
location: class Bug570891a.validationproblem.impl.Duck3Impl
Solution. The synthesis of Duck3 should be aware of what the renaming is and so should respect the global rather than local renaming.
Workaround:
for simple models: ensure all inheritances are consistently first or not-first
for complex models: introduce a dummy first inheritnace to force all to be renamed
The text was updated successfully, but these errors were encountered: