Bug 544629 - Null-safe navigation corollaries
Summary: Null-safe navigation corollaries
Status: NEW
Alias: None
Product: QVTd
Classification: Modeling
Component: Core (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 544653
Blocks:
  Show dependency tree
 
Reported: 2019-02-20 07:49 EST by Ed Willink CLA
Modified: 2020-06-07 13:02 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Willink CLA 2019-02-20 07:49:16 EST
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.
Comment 1 Ed Willink CLA 2019-02-21 03:44:44 EST
(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.
Comment 2 Ed Willink CLA 2019-02-23 03:12:07 EST
(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
- add an isDeclarative() predicate to the OCL validation, always true
b1) A CallExp.isDeclarative can be set by the QVTr parser
b2) A DeclarativeOperationCallExp etc overrides isDeclarative() and is used by the QVTr parser for the differing semantics.

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?
Comment 3 Ed Willink CLA 2019-02-23 03:29:48 EST
(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.
Comment 4 Ed Willink CLA 2019-02-24 06:12:31 EST
(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.
Comment 5 Ed Willink CLA 2019-02-24 06:53:54 EST
(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.
Comment 6 Ed Willink CLA 2019-11-07 03:43:01 EST
> (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
Comment 7 Ed Willink CLA 2020-06-07 05:14:14 EDT
(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.
Comment 8 Ed Willink CLA 2020-06-07 13:02:03 EDT
(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.