[validation] Conformance to templated parameter lower bound not checked

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 485151 | | Status | NEW | | Importance | P3 normal | | Reported | Jan 04, 2016 10:15 EDT | | Modified | Nov 16, 2016 13:28 EDT | | Blocks | 507628 | | Reporter | Adolfo Sanchez-Barbudo Herrera |

Description

Working on the the richer classescs2asV2Lookup.ocl [1]

The following OCL expression produces a CGed LookupVisitor implementation with compilation errors:

context Class\ def : _env(child : ocl::OclElement) : lookup::LookupEnvironment =\ parentEnv().nestedEnv() .addElements(let allSuperClasses = self->closure(superClass)\ in allSuperClasses->collect(ownedProperties)\ ->includingAll(allSuperClasses.ownedOperations)\ ->asOrderedSet() -- FIXME ) .nestedEnv() .addElements(ownedOperations) .addElements(ownedProperties)

Steps to reproduce coming soon.

[1] org.eclipse.qvtd.cs2as.compiler.tests\src\org\eclipse\qvtd\cs2as\compiler\tests\models\example2\classescs2asV2Lookup.ocl

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jan 04, 2016 10:37

Branches:\ OCL -> I201601021212\ QVTd -> asanchez/485151

Step to reproduce:\ 1 - Execute GenerateExample2Models.mwe2 [1]\ 2 - AbstractClassesUnqualifiedLookupVisitor has compilation errors

In order to check which is the specific change that makes that generation fail, please compare the lookup ocl file [2] with respect to its previous version.

Note that if I do manual tuning to make the compilation error disappear all OCL2QVTi test cases pass (the interesting ones are testExample2_V2_Interpreted and testExample2_V2_CG)

[1] org.eclipse.qvtd.cs2as.compiler.tests\src\org\eclipse\qvtd\cs2as\compiler\tests\models\example2\mwe\GenerateExample2Models.mwe2\ [2] org.eclipse.qvtd.cs2as.compiler.tests\src\org\eclipse\qvtd\cs2as\compiler\tests\models\example2\classescs2asV2Lookup.ocl

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jan 14, 2016 09:38

Apparently, I hadn't pushed asanchez/485151 ??

I've rebased asanchez/485151 on top of newest stable branch.

I'm seeing an additional CG issue:

• LookupEnvironment [1] also has an incorrect generation issue. It may merit its own bugzilla, but perhaps you want to fix everything at once (the steps in comment 1 reproduce both compilation errors)

[1] org.eclipse.qvtd.cs2as.compiler.tests\tests-gen\example2\classes\lookup\impl\LookupEnvironmentImpl.java

[eclipse-ocl-bot]

By Ed Willink on Jan 15, 2016 14:54

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #2)

I'm seeing an additional CG issue:

• LookupEnvironment [1] also has an incorrect generation issue. It may merit its own bugzilla, but perhaps you want to fix everything at once (the steps in comment 1 reproduce both compilation errors)

This is fixed by Bug485965.

---

The original issue is visible in the Complete OCL hovertext. Hovering over the "->" on the line above the FIXME shows:

OperationCallExp Bag(classes::Property)::includingAll(Bag(classes::Operation)) : Bag(classes::Element)

which is exactly what the CG does. Why wasn't NamedElement (or TypedElement) identifies as the common specialization?

Unlike OCL, classes::TypedElement does not extend NamedElement, so the type determination has two equally good choices, NamedElement or TypedElement. Maybe it confused and went for the common type between the choices.

[eclipse-ocl-bot]

By Ed Willink on Jan 15, 2016 15:55

(In reply to Ed Willink from comment #3)

Maybe it confused and went for the common type between the choices.

AbstractInheritance::getCommonInheritance is specifically commented for the ambiguous common super type case:

// More than one so must go less deep to find uniqueness

// Must be unique to avoid arbitrary choice for e.g. Sequence{1, 2.0, '3'}->elementType

Arbitrary choices are certainly bad, but skipping both valid choices is worse.

An arbitrary choice can at least be deterministic by an alphabetic sort. (Inheritance entries are already sorted.)

Internally we could add some complexity with a multi-type to defer the arbitrary choice only one of which might support a later type requirement. Doesn't help this example.

---

Or we throw the problem back on the user to disambiguate explicitly. Seems a better solution. Then we just have the validation bug that the lower bound violation does not appear in the editor.

[eclipse-ocl-bot]

By Ed Willink on Jan 15, 2016 15:59

Changing

in allSuperClasses->collect(ownedProperties)\ ->includingAll(allSuperClasses.ownedOperations)\ ->asOrderedSet() -- FIXME

to

in let properties : Bag(NamedElement) = allSuperClasses->collect(ownedProperties)\ in properties->includingAll(allSuperClasses.ownedOperations)\ ->asOrderedSet()

and the CG is error free.

[eclipse-ocl-bot]

By Ed Willink on Jan 16, 2016 09:14

commit 438de1aece8ee81e856b73e405f12a929cc0e65f pushed to master for M5

fixes the original report. The lack of validation remains.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jan 17, 2016 09:33

Hi Ed,

I understand your comments (excepting "the validation bug that the lower bound violation does not appear in the editor").

However, there is still something smelly.

By analyzing the text hover, prior the the expression rework, the type of the variable init expression is "Bag(Element)", after the rework is "Bag(NamedElement)".

This means, that the type of the init expression varies depending on which is the declared type of the let variable.

To me this is clearly unsound. The ocl expression of the init expression should be always the same, regardless of what is around the OCL expression, just changes to the context variable (self). The additional let variable type specification, should act as additional check between the variable and the init expression, and it should not impact on anything of the init expression.

[eclipse-ocl-bot]

By Ed Willink on Jan 17, 2016 09:38

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #7)

the variable init expression is "Bag(Element)", after the rework is "Bag(NamedElement)".

The rework involves an explicit rather than inferred type, thereby allowing the ambiguous common type to be used.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jan 17, 2016 11:13

(In reply to Ed Willink from comment #8)

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #7)

the variable init expression is "Bag(Element)", after the rework is "Bag(NamedElement)".

The rework involves an explicit rather than inferred type, thereby allowing the ambiguous common type to be used.

Yes, I understand what is happening. I simply don't agree with it, hence my claim that it's unsound.

The inferred type comes by inferring the variable type from the init expression, that's OK... What is wrong is having the init expression to be different depending on whether the variable has explicitly declared the type or hasn't done so.

[eclipse-ocl-bot]

By Ed Willink on Jan 17, 2016 11:35

I'm not convinced that you do understnad. You certainly need to make your point much more cleary.

In my view it is just a case of an explicit declaration being able to achieve something that an inferred declaration cannot.

This can occur in Java too. e.g.

 = Collections.<ExplicitType>emptyList();

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jan 17, 2016 11:50

[Trying to make the point clearer]

let varName : varType = varInitExp

My point is that varInitExp is an OCL expression in its own, which SHOULD be independent of the left hand side var declaration. Whatever varType is in the lefthandside, it SHOULD not ever drive whatever an OCL expression is afterwards.

Your java example doesn't apply, because the template specification is clearly part of the expression.

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Jan 17, 2016 12:00

You are not looking at the example carefully enough.

let properties : Bag(NamedElement) = allSuperClasses->collect(ownedProperties)\ in properties->includingAll(allSuperClasses.ownedOperations)\ ->asOrderedSet()

The explicit declaration is

let properties : Bag(NamedElement) = allSuperClasses->collect(ownedProperties)

which refines the Bag(Property) declaration of the first original term to Bag(NamedElement) so that the subsequent

properties->includingAll(allSuperClasses.ownedOperations)

has no ambiguity.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jan 17, 2016 12:13

Yes, you are right. My comments are a red herring. I got distracted with the nested letExp.

Regards,\ Adolfo.