eclipse-uml2 / uml2

An EMF-based implementation of the UML 2.x metamodel for the Eclipse platform.
Eclipse Public License 2.0
5 stars 4 forks source link

Inconsistent multiple inheritance renaming #108

Open eclipse-uml2-bot opened 5 hours ago

eclipse-uml2-bot commented 5 hours ago

| --- | --- | | 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

eclipse-uml2-bot commented 5 hours ago

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.

eclipse-uml2-bot commented 5 hours ago

By Ed Willink on Feb 08, 2021 04:13

Created attachment 285475 OCL-free repro

(In reply to Ed Willink from comment #1)

A first attempt to trim the example to a smaller OCL-free repro failed.

Seems a bit temperamental, but eventually the attached generates a bad Duck3.Impl.

:compression: Bug571005.zip

eclipse-uml2-bot commented 5 hours ago

By Ed Willink on Feb 08, 2021 04:24

(In reply to Ed Willink from comment #2)

Seems a bit temperamental,

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).

but eventually the attached generates a bad Duck3.Impl.

seems to come and go as

<details key="DUPLICATE_FEATURES" value="PROCESS"/>

is changed. DISCARD ok, PROCESS bad.

eclipse-uml2-bot commented 5 hours ago

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:\ Description Resource Path Location Type\ The method basicGetBase_NamedElement() of type Duck3Impl must override or implement a supertype method Duck3Impl.java /com.validationproblem.profile/src-gen/com/validationproblem/profile/validationproblem/impl line 85 Java Problem

eclipse-uml2-bot commented 5 hours ago

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.

eclipse-uml2-bot commented 5 hours ago

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.

eclipse-uml2-bot commented 5 hours ago

By Ed Willink on Feb 09, 2021 00:47

Surely the consistently first / not-first workaround is easy enough to arrange?

eclipse-uml2-bot commented 5 hours ago

By Deniz Eren on Feb 09, 2021 02:03

Hi Ed,\ Not sure if it's that easy, when I change the generalization order or the redefined property order for the base_* property in the model it doesn't have an impact, so I wasn't able to determine what decides which order is taken at the code generator phase. Also, when there are 3 instead of 2 generalization it gets more interesting, but anyway, I have a way forward now, but it does involve editing the generated code. I also have all the genmodel options to Process except our Camel Case Names option.

eclipse-uml2-bot commented 5 hours ago

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.

eclipse-uml2-bot commented 5 hours ago

By Deniz Eren on Feb 09, 2021 05:46

Yes that’s a good idea

eclipse-uml2-bot commented 5 hours ago

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?

eclipse-uml2-bot commented 5 hours ago

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();

eclipse-uml2-bot commented 5 hours ago

By Deniz Eren on Feb 09, 2021 08:35

Of course this can be fixed with a simple cast; but there are 90+ instances...

eclipse-uml2-bot commented 5 hours ago

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.

eclipse-uml2-bot commented 5 hours ago

By Ed Willink on Feb 09, 2021 13:12

(In reply to Deniz Eren from comment #12)

final /@NonInvalid/ Comment base_Comment_0 = aDocumentSection_0.getBase_Element();

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.

eclipse-uml2-bot commented 5 hours ago

By Deniz Eren on Feb 09, 2021 19:32

Hi Ed,\ Here is the bug ticket: Bug 571074

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:\ Changed the GenModel GenAnnotation:\

\ to DISCARD.

eclipse-uml2-bot commented 5 hours ago

By Deniz Eren on Feb 09, 2021 20:02

Hi Ed,\ Please also see extension issue, regarding missing "import" in Java code: Bug 571075

eclipse-uml2-bot commented 5 hours ago

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).

eclipse-uml2-bot commented 5 hours ago

By Ed Willink on Feb 23, 2021 05:50

(In reply to Ed Willink from comment #3)

seems to come and go as

<details key="DUPLICATE_FEATURES" value="PROCESS"/>

is changed. DISCARD ok, PROCESS bad.

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.)

eclipse-uml2-bot commented 5 hours ago

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?

eclipse-uml2-bot commented 5 hours ago

By Ed Willink on Feb 23, 2021 06:58

(In reply to Deniz Eren from comment #20)

selectByKind and oclIsKindOf

Definitely not a UML issue. Please raise an OCL Bugzilla with repro.

eclipse-uml2-bot commented 5 hours ago

By Deniz Eren on Feb 23, 2021 07:24

Let's see if I can replicate it with simple test profile..

eclipse-uml2-bot commented 5 hours ago

By Deniz Eren on Feb 24, 2021 00:54

The issue regarding selectByKind isn't easy to replicate with small model.\ I've been testing with the latest build:\ Build #258 (22 Feb 2021, 11:51:05)\ Version: 6.14.0.v20210222-1651

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:\ variable size: 8\ ECORE_selectByKind size: 0

Other times prints:\ variable size: 8\ ECORE_selectByKind size: 8

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:\ try {\ Thread.sleep(5000);\ } catch (InterruptedException e) {\ // TODO Auto-generated catch block\ e.printStackTrace();\ }

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?

eclipse-uml2-bot commented 5 hours ago

By Deniz Eren on Feb 24, 2021 01:10

P.S. with this new version: Build #258 (22 Feb 2021, 11:51:05)\ Version: 6.14.0.v20210222-1651 - I do not need to manually edit anything! Looking good - only this new memory leak / race condition described above exists.

eclipse-uml2-bot commented 5 hours ago

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.

eclipse-uml2-bot commented 5 hours ago

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?

eclipse-uml2-bot commented 5 hours ago

By Ed Willink on Feb 26, 2021 05:01

(In reply to Ed Willink from comment #19)

(Bug 571407 identified this need for same-named/new-named redefinition distinction when sythesizing Jav for OCL code.)

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.

eclipse-uml2-bot commented 5 hours ago

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.

eclipse-uml2-bot commented 5 hours ago

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.