[pivot] oclIsKindOf/selectByKind fail if the argument is not a literal

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 580139 | | Status | NEW | | Importance | P3 normal | | Reported | Jun 11, 2022 03:09 EDT | | Modified | Jun 20, 2022 06:36 EDT | | Depends on | 580140 | | See also | 580136, 423905, Gerrit change 194286, Gerrit change 194288, Git commit 4876ef76 | | Reporter | Ed Willink |

Description

From Bug 580136#c1

Literals seem really broken

InternationalizedProfile::InEnglish.name => 'InEnglish'\ let st : Stereotype = InternationalizedProfile::InEnglish in st.name =>

[eclipse-ocl-bot]

By Ed Willink on Jun 11, 2022 03:19

Simpler failure:

let t : ocl::OclType = Integer in 4.oclIsKindOf(t)

gives

Multiple markers at this line

• Unresolved Operation 'Integer::oclIsKindOf(t)'
• Unresolved Property 'Integer::t'

[eclipse-ocl-bot]

By Ed Willink on Jun 11, 2022 04:16

Simple fix. EssentialOCLCSLeft2RightVisitor.resolveOperationArgumentTypes correctly augments the name resolution search for a Type-typed argument to consider a type literal, but occludes the fall back to the normal expresson evaluation that would resolve the VariableExp.

Trivial fix. Fall-back if no type found.

But why the special search? Commenting out and it works fine. Seems like the general parsing of TypeExp improved else the use of lookupType was misguided. No. For examples using "Integer" the NameExpCS is indeed a TypeLiteralExpCS and so easy to resolve. But for general user classes we want to bias oclIsKindOf(MyClass) directly to a type rather than have to disambiguate a type/property/variable conflict in idiot code.

[eclipse-ocl-bot]

By Ed Willink on Jun 11, 2022 06:12

Pushed to maintenance/6.17.2 for 6.17.2. N-build imminent.

http://www.eclipse.org/modeling/download.php?file=/modeling/mdt/ocl/downloads/drops/6.17.2/N202206110946/mdt-ocl-Update-N202206110946.zip available.

[eclipse-ocl-bot]

By Ed Willink on Jun 12, 2022 03:47

(In reply to Ed Willink from comment #2)

Trivial fix. Fall-back if no type found.

No. Works fine for simple tests with built-in types. Once we progress Bug 580136 with user types, we get a problem through re-use of the Xtext proxy resolution, once to lookup a Type, then to fall-back and lookup a TypedElement. Once we reset the PathName between searches the parse 'succeeds' but only after dropping an unresolved type diagnostic.

But why the special search? Commenting out and it works fine.

Indeed, we just need to ensure that the regular search is configured for Type or TypedElement.

[eclipse-ocl-bot]

By Ed Willink on Jun 13, 2022 00:13

(In reply to Ed Willink from comment #4)

Indeed, we just need to ensure that the regular search is configured for Type or TypedElement.

But writing tests for this seem to hit a test harness deficency. Bug 580140.

[eclipse-ocl-bot]

By Ed Willink on Jun 13, 2022 11:58

(In reply to Ed Willink from comment #2)

But why the special search?

A Type-typed argument is at the wrong meta-level.

Normally "x" is a variable/parameter "x" or a property "self.x" all at the evaluation meta-level.

However, in "oclIsKindOf(x)", "x" should be a type, so it is sensible to find the resolution to the type "x" rather than an expression, without forcing the user to write "q::x". This is the same as "Set(x)" rather than "Set(q::x)". (Note that type names from the meta-level are often 'valid' as implicit opposite names at the evaluation meta-level and we never want them as type arguments.)

When a let-variable is used we must write "let y = q::x in oclIsKindOf(y)" since we cannot help the user by exploiting knowledge that a type is required.

It would be less confusing to require "oclIsKindOf(q::x)" but then we would also need to require "Set(q::x)".

(In reply to Ed Willink from comment #4)

Indeed, we just need to ensure that the regular search is configured for Type or TypedElement.

Precisely. Regular TypedElements with an optional occlusion by Type from the meta-level.

Only a couple of test depend on this functinality. Most notably in Pivot.ocl

context Element\
def: allOwnedElements() : Set(Element) =\
self->closure(oclContents()->selectByKind(Element))

Element must not be resolved via the iterator.

[eclipse-ocl-bot]

By Ed Willink on Jun 13, 2022 13:20

(In reply to Ed Willink from comment #6)

It would be less confusing to require "oclIsKindOf(q::x)" but then we would also need to require "Set(q::x)".

We already require "Set(q::x)". The following fails without a qualifier.

ocl.assertQueryEquals(train1, null, "let t : Set(Train) = null in t");

---

cf. Java

a) No qualifier is needed for a type literal; there is a ".class" suffix.

b) No qualifier is needed for instanceof; known type context, only literals.

c) No qualifier is needed for Set; known type context using import.

OCL.

a) Hard to avoid qualifier since implicit opposites offer too many ambiguities.

b) Hard to avoid qualifier since oclIsKindOf may receive an expression.

c) Hard to avoid qualifier since we don't really have an import.

Seems we're trying to be too clever and assuming that the clever type context sensitivity already works when it doesn't.

[eclipse-ocl-bot]

By Ed Willink on Jun 14, 2022 00:29

(In reply to Ed Willink from comment #7)

Seems we're trying to be too clever and assuming that the clever type context sensitivity already works when it doesn't.

Except that sometimes it does. In Pivot.ocl

context Element\ def: allOwnedElements() : Set(Element) =\ self->closure(oclContents()->selectByKind(Element))

needs a qualifier to avoid an implicit opposite of the iterator being resolved.

Smilarly in QVTimperative.ocl

context SetStatement\ inv ValueDoesNotNavigateFromRealizedVariables:\ ownedExpression->closure(e : ocl::OclElement | e.oclContents())->selectByKind(ocl::VariableExp)->select(referredVariable.oclIsKindOf(NewStatement))->select(s | s.oclContainer().oclIsKindOf(ocl::CallExp) and s.oclContainer().oclAsType(ocl::CallExp).ownedSource = s)->isEmpty()

NewStatement needs a qualifier.

So this simplification is a breaking change; rare but far from never or only stupid.

How do we lookup a Type and a TypedElement concurrently when the ancestor ascent looks for class content before climbing to package content?

And a new hazard. When looking up a package/type in the context of an implicit source, is the package/type located in the scope of the implicit source or of the outer self? Currently we use the implicit source. Crazy, types must be consistently spelled throughput an expression.

So the concurrent lookup should lookup Type first when expected, before any attempt at TypedElement is made. Just what the current code does. We just need to do this within a single proxy resolution rather than a cascade.

[eclipse-ocl-bot]

By Ed Willink on Jun 14, 2022 07:26

(In reply to Ed Willink from comment #8)

So the concurrent lookup should lookup Type first when expected

This is a fudge. For "source.oclIsKindOf(T)", "T" may legitimately be

a) the type T in the package of source\ b) the type T in the package of self\ c) a parameter named T with a Type value\ d) a let-variable named T with a Type value\ e) a property named T with a Type value\ f) an implicit-opposite-property named T with a Type value

More generally, the argument may be an arbitrary Type-typed expression.

We must find all of these and report an ambiguity unless a reasonable heuristic resolves the ambiguity. Apply the heuristic after the ambiguity rather than search pragmatically by applying an EClass lookup filter to search for any TYPE rather than any ELEMENT; the TYPE search misses variables/parameters/properties.

So fix the current tests to find all the candidates and then refine the disambiguation heuristsic so that hopefully everything works as is and the new let...oclIsKindOf tests work too.

[eclipse-ocl-bot]

By Ed Willink on Jun 16, 2022 05:10

(In reply to Ed Willink from comment #9)

So fix the current tests to find all the candidates and then refine the disambiguation heuristsic so that hopefully everything works as is and the new let...oclIsKindOf tests work too.

No. The problem is exemplified by part of a QVTi invariant

select(referredVariable.oclIsKindOf(NewStatement))

which is an ambiguously short for

select(referredVariable.oclIsKindOf(qvtimperative::NewStatement))\
select(ve : VariableExp | referredVariable.oclIsKindOf(ve.NewStatement))

Problem. Names are resolved by a CST ascent allowing each level to add names. The search finishes when the 'final' result is found.

The implicit opposite solution is found as the ascent traverses the IteratorExp and considers the type of the iterator which has an implicit opposite.

The type solution is available once the ascent reaches the outer ContextCS and uses the package of the type of the context variable to find types.

Old (bad) solution:

Do two searches for Type-typed arguments. Search one just for types, then search two for elements. The Type search finds the 99% use case of type literals ok. The element search finds the fall backs, but only after the Type search has polluted the errors.

A better (bad) solution:

Search for types concurrently. But this means that the types must be found at the iterator where the context package is unknown. Potentially requires a new API to do two passes through the Attribution hierarchy.

New (good) solution:

The problem is terminating once the 'final' solution is found. Could do everything and disambiguate, but there is an unpleasant amount of functionality that might change. Simpler to just redefine 'final' result to consider only explicit names as search terminating. Implicit names are found but the search continues, so we end up with the ambiguities that are resolved by the prefer-no-implicit disambiguating heuristic.

Residual gotcha:

ATL2QVTr does "oclAsType(pivotMM::VariableDeclaration[1])" that is now detected as an AssociationClassCallExp. New EssentialOCLCSLeft2RightVisitor::isAssociationClassCallExp predicates detects numeric qualifiers The inadequacies of qualified association parsing are Bug 423905.

[eclipse-ocl-bot]

By Ed Willink on Jun 18, 2022 10:59

Another gotcha:

NavigationOperatorCSAttribution makes one attempt for non-static addElementsOfScope then for a static addElementsOfScope so a nested lookup may make final decisions too soon. These static prooblems should go away once the faulty static declarations are fixed by the reworking of static.

[eclipse-ocl-bot]

Jun 20, 2022 02:52

New Gerrit change created: https://git.eclipse.org/r/c/ocl/org.eclipse.ocl/+/194286

[eclipse-ocl-bot]

Jun 20, 2022 02:52

New Gerrit change created: https://git.eclipse.org/r/c/ocl/org.eclipse.ocl/+/194288

[eclipse-ocl-bot]

Jun 20, 2022 06:36

Gerrit change https://git.eclipse.org/r/c/ocl/org.eclipse.ocl/+/194286 was merged to [master].\ Commit: http://git.eclipse.org/c/ocl/org.eclipse.ocl.git/commit/?id=4876ef76c5116f86ec3e513ffcf4f06e68e5cb92