eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

[cg] Redundant CGLetExp corrupts ShadowExp #1753

Open eclipse-ocl-bot opened 3 weeks ago

eclipse-ocl-bot commented 3 weeks ago

| --- | --- | | Bugzilla Link | 506656 | | Status | NEW | | Importance | P3 normal | | Reported | Oct 27, 2016 15:48 EDT | | Modified | Oct 27, 2016 17:45 EDT | | Reporter | Ed Willink |

Description

Bug 506388 describes the erroneous generation of

CTORid_rightOperand.initValue(symbol_2, final /@Thrown/ org.xtext.example.delphi.delphi.@org.eclipse.jdt.annotation.Nullable simpleExpression self_28 = self_22.getRight();\ if (self_28 == null) {\ throw new InvalidValueException("Null where non-null value required");\ } );

rather than

final /@Thrown/ org.xtext.example.delphi.delphi.@org.eclipse.jdt.annotation.Nullable simpleExpression self_28 = self_22.getRight();\ if (self_28 == null) {\ throw new InvalidValueException("Null where non-null value required");\ }\ CTORid_rightOperand.initValue(symbol_2, self_28);

as a result of the use of

rightOperand <- let self = $GUARD(self.right) in (null)

which is a bit confusing to read. It is legitimately

rightOperand <- let var1 = $GUARD(var2.right) in var1.ast()

where "var1.ast()" is a final operation whose value is null.

Null is detected as inlineable for the

CTORid_rightOperand.initValue(symbol_2, "null")

but the redundant let needlessly wraps the "null".

eclipse-ocl-bot commented 3 weeks ago

By Ed Willink on Oct 27, 2016 17:23

Attempting a simple "let ... in null" repro fails because the AnalysisVisitor detects that "null" is constant and discards the "let ...in". Need a more complex "null" such as an inlined operation.


We could/should run another AnalysisVisitor / ConstantFolder after inlining.

But there is a more insidious problem, also Bug 506630, that the family of CG*Impl predicates such as isInlineable are manual projections of an underlying characteristic. Is there a way to automate these projections?


Neither of the comment 0 formulations is plausible, since both lose the ultimate null value.

eclipse-ocl-bot commented 3 weeks ago

By Ed Willink on Oct 27, 2016 17:45

Stepping the original repro to see if the bad CGLetExp serialization can be fixed highlights another problem.

A single CG2Java visitor serves two different purposes:

The mode switching is too-clever; an infectious complexity. Not obvious how to add more complexity to solve this.

Probably better to unwind the complexity with a distinct append symbol name visitor.