[CS2AS] Incorrect serialization of the safe navigation operator

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 485696 | | Status | RESOLVED FIXED | | Importance | P3 normal | | Reported | Jan 12, 2016 13:28 EDT | | Modified | Jan 13, 2016 07:46 EDT | | Reporter | Adolfo Sanchez-Barbudo Herrera |

Description

When working on Bug 485668, I got confused when visualizing a .qvtias file obtained from the OCL2QVTp transformation.

From the classescs2asV2.ocl[1], which generates classescs2asV2.qvtp.qvtias [2], I devise the following incorrect mismatches:

ownedNameExp?.ast() -> nameExpCS.ownedNameExp.ast?.oclAsType(as::CallExp);

ast().oclAsType(as::OperationCallExp).referredOperation?.type -> nameExpCS.ast.oclAsType(as::CallExp).oclAsType(as::OperationCallExp).referredOperation.type;

Indeed, it seems to be a bug in the logic which produce the CS from the AS, with respect to the isSafe CallExp flag. The flag is used from the CallExp source, whereas it should be considered the flag from the own CallExp which is being converted.

Fix coming next.

[1] org.eclipse.qvtd.cs2as.compiler.tests\src\org\eclipse\qvtd\cs2as\compiler\tests\models\example2\classescs2asV2.ocl\ [2] org.eclipse.qvtd.cs2as.compiler.tests\src\org\eclipse\qvtd\cs2as\compiler\tests\models\example2\classescs2asV2.qvtp.qvtias

[eclipse-ocl-bot]

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

asanchez/485696 branch contributes a potential fix. So that now I visualize the expected outputs:

ownedNameExp?.ast() -> nameExpCS.ownedNameExp?.ast.oclAsType(as::CallExp);

ast().oclAsType(as::OperationCallExp).referredOperation?.type -> nameExpCS.ast.oclAsType(as::CallExp).oclAsType(as::OperationCallExp).referredOperation?.type;

From QVTd, I suggest to use asanchez/485668 and execute for instance OCL2QVTiTestCases::testNewExample2_V2_Interpreted, in order to generate the mentioned classescs2asV2.qvtp.qvtias file and reliably experience the wrong visualization as described by comment 0

[eclipse-ocl-bot]

By Ed Willink on Jan 13, 2016 07:05

At the heart of the problem is the over-complicated 'omniscient'

boolean isSafe = (asSource instanceof CallExp) && (((CallExp)asSource).isIsSafe() || ((asSource.eContainer() instanceof IteratorExp) && (((IteratorExp)asSource.eContainer()).isIsSafe())));

This looks smelly through the use of two different flags: asSource.isIsSafe() and asSource.eContainer().isIsSafe().

The special case for IteratorExp looks like a 'quick fix'.

Your solution avoids the inflexible 'omniscience' but pushes the problem to the caller.

The flag is used from the CallExp source, whereas it should be considered the flag from the own CallExp which is being converted.

I suspect that is is enough just to change to:

boolean isSafe = (asSource != null) && (asSource.eContainer() instanceof CallExp) && (((CallExp)asSource.eContainer()).isIsSafe()));

This accurately predicts the new argument for testNewExample2_V2_Interpreted and those of all OCL JUnit tests.

[eclipse-ocl-bot]

By Ed Willink on Jan 13, 2016 07:34

Your fix with an extra argument also demonstrates that the signature of

protected @NonNull ExpCS createNavigationOperatorCS(@Null OCLExpression asSource, @NonNull ExpCS csArgument, boolean isConverted) {

is so unhelpful as to be wrong. Change to

protected @NonNull ExpCS createNavigationOperatorCS(@NonNull CallExp asCallExp, @NonNull ExpCS csArgument, boolean isConverted) {\ OCLExpression asSource = asCallExp.getOwnedSource();\ boolean isSafe = asCallExp.isIsSafe();

and the complexity vanishes.

Pushed to master for M5.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jan 13, 2016 07:43

Yes, passing the CallExp is the more appropriate. Note that the change is not backwards compatible, though.

I tend to break as minimum as possible, and let you play with the code (as you often do) as you feel is more appropriate.

[eclipse-ocl-bot]

By Ed Willink on Jan 13, 2016 07:46

Fortunately it was a protected 'xtext' method not re-used by QVTd.