search

eclipse-qvtd / org.eclipse.qvtd

Eclipse Public License 2.0
0 stars 0 forks source link

Null-safe navigation corollaries #365

Open eclipse-qvtd-bot opened 2 weeks ago

eclipse-qvtd-bot commented 2 weeks ago

| --- | --- | | Bugzilla Link | 544629 | | Status | NEW | | Importance | P3 normal | | Reported | Feb 20, 2019 07:49 EDT | | Modified | Jun 07, 2020 13:02 EDT | | Depends on | 544653 | | See also | 544652, 550602, 543602 | | Reporter | Ed Willink |

Description

Improved OCL null-safe tooling results in many tens of warnings in RelToCore etc since there are many navigations from possibly null properties.

We have a design choice.

Is the bad navigation just another form of pattern match that therefore fails for null sources?

Is the bad navigation just bad OCL that the user should be alerted to?

Both are valid. Perhaps we need to see how painful the bad OCL warnings are.

eclipse-qvtd-bot commented 2 weeks ago

By Ed Willink on Feb 21, 2019 03:44

(In reply to Ed Willink from comment #0)

Both are valid. Perhaps we need to see how painful the bad OCL warnings are.

Yes, but a QVTr pattern is a truth statement. A null-navigation is not part of a truth. The opacity of what matches may be helped by a missing match assistant, see Bug 544652.

QVTr must therefore suppress the OCL null-safe diagnosis. Currently the FlowAnalysis is not extensible; see Bug 544653.

Suppression is clearly appropriate in match expressions, but are there some calculations such as operation arguments that are more legitimately regarded as OCL compuations rather than matches? If a null navigation occurs within an operation, the operation execution fails at run-time causing the relation to fail. The same applies to operation arguments.

But these are dirty failures; much better to exclude them at compile time. The CG should synthesize an early not-null guard to avoid exceptions. The UI could offer INFO blue squiggles for null-guarded navigations. A preference could change them to red/yellow. A rule/transformation directive could assert null safety requiring all null hazards to be explicitly matched away.

eclipse-qvtd-bot commented 2 weeks ago

By Ed Willink on Feb 23, 2019 03:12

(In reply to Ed Willink from comment #1)

QVTr must therefore suppress the OCL null-safe diagnosis. Currently the FlowAnalysis is not extensible; see Bug 544653.

Not sure that this helps. Redundant ? is still a problem. It is just missing "?" that changes semantics to match.

Options:

a) modify the preferences to suppress the prolematic validations

b) modify the validations to be 'isDeclarative' aware

c) override the validation in e.g. DeclarativeOperationCallExp

a) is klunky, c) is non-LSP

CallExp.isDeclarative is easy but an OCL extension of state.

DeclarativeOperationCallExp is a bit messy but only an OCL extension of behaviour.

Is there a more scalably extensible way?

eclipse-qvtd-bot commented 2 weeks ago

By Ed Willink on Feb 23, 2019 03:29

(In reply to Ed Willink from comment #2)

DeclarativeOperationCallExp is a bit messy but only an OCL extension

The problem is that a QVTr OperationCallExp is-not-a OCL OperationCallExp and so direct re-use of or inheritance from is broken. This could be fixed by {QVTr,OCL}OperationCallExp extending AbstractOperationCallExp so that there is no semantic conflict. But everything in OCL needs to have a concrete OCLxxx layer with validation and an Abstractxxx layer to support variant derived sematics.

Is there a more scalably extensible way?

(In reply to Ed Willink from comment #2)

(In reply to Ed Willink from comment #1)

QVTr must therefore suppress the OCL null-safe diagnosis. Currently the FlowAnalysis is not extensible; see Bug 544653.

Not sure that this helps.

Yes it does. The problem is that in "x.y", "x" may-be-null in OCL, but "x" is-non-null in QVTr. The QVTr FlowAnalysis can perform the hierarchical analysis to discover that "x" comes via a template-expression that gurantees that "x" is not null when "x.y" executes. OperationCallExp semantics are respected, validation rule does not need changing. Just the flow-analysis to support the validation and the CG to perform the extra null-guard.

eclipse-qvtd-bot commented 2 weeks ago

By Ed Willink on Feb 24, 2019 06:12

(In reply to Ed Willink from comment #0)

Both are valid. Perhaps we need to see how painful the bad OCL warnings are.

Peparing to tune up the derived QVTrelationFlowAnalysis for RElToCore.qvtr reveals that most of the unsafe navigations are actually bugs for which imposing a silent no-execution relaizes a defective transformation.

e.g.

whenVars = r.when.bindsTo;

should be

whenVars = if r.when <> null then r.when.bindsTo else Set{} endif;

which cannot be the current safe navigation since it converts an absence to an empty-presence.

But it's actually

whenVars = r.when->collect(bindsTo);

which is valid for a null collection.

It must be the PropertyCallExp::UnsafeSourceCanNotBeNull constraint that is broken for null collections.

eclipse-qvtd-bot commented 2 weeks ago

By Ed Willink on Feb 24, 2019 06:53

(In reply to Ed Willink from comment #4)

But it's actually

whenVars = r.when->collect(bindsTo);

No. Relation::when is a Pattern[?]. It may legitimately (usually) be null. whenVars is a computation not matched variable. Treating it as a match cripples the relation.

Taking some control as:

    rWhen : qvtbase::Pattern[1] = r.when;\
    whenVars = rWhen.bindsTo;

does not help. Now the rWhen initializer is incompatible.

It requires an explicit cast

    rWhen = r.when.oclAsType(qvtbase::Pattern[1]);\
    whenVars = rWhen.bindsTo;

to make the warning go away. And this is clearly a pattern match.

(In reply to Ed Willink from comment #0)

Both are valid. Perhaps we need to see how painful the bad OCL warnings are.

Maybe the current behaviour is good.

eclipse-qvtd-bot commented 2 weeks ago

By Ed Willink on Nov 07, 2019 03:43

(In reply to Ed Willink from comment #0)

Both are valid. Perhaps we need to see how painful the bad OCL warnings are.

Maybe the current behaviour is good.

See Bug 550602

eclipse-qvtd-bot commented 2 weeks ago

By Ed Willink on Jun 07, 2020 05:14

(In reply to Ed Willink from comment #3)

Just the flow-analysis to support the validation and the CG to perform the extra null-guard.

This occurs all over again with the new SymbolicEvaluationVisitor which is potentially much more precise. 2 QVTc and 14 QVTr tests have validation failures.

It is not entrely clear why the FlowAnalysis was ok.

One challenge is in Families2Persons where in

::Families2Persons::familyName(Families::Member[1]) : String[1]\ if self.familyFather.oclIsUndefined().not()\ then self.familyFather.lastName\ else if self.familyMother.oclIsUndefined().not()\ then self.familyMother.lastName\ else if self.familySon.oclIsUndefined().not()\ then self.familySon.lastName\ else self.familyDaughter.lastName\ endif endif endif

self.familyFather is not-null folding the functionality. THis is a straight OCL query so must just be a bug.

eclipse-qvtd-bot commented 2 weeks ago

By Ed Willink on Jun 07, 2020 13:02

(In reply to Ed Willink from comment #7)

so must just be a bug.

Doh! Missing dedice / reverse evaluation support for oclIsUndefined().

But the fourth alternative is a genuine hazard without the plausible knowledge that one of self.familyFather/familyFather/familyMother/familyDaughter is non-null.

If the user provided a pre-condition/invariant this might be symbolically evaluatable. Not pre-condition - it just pushes the problem sideways to a different requirement. But an invariant introduces a new truth.