Bug 425066 - Enforce adding of mapping results to model extents
Summary: Enforce adding of mapping results to model extents
Status: NEW
Alias: None
Product: QVTo
Classification: Modeling
Component: Engine (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 487824
Blocks:
  Show dependency tree
 
Reported: 2014-01-08 03:17 EST by Christopher Gerking CLA
Modified: 2016-02-22 10:14 EST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Gerking CLA 2014-01-08 03:17:06 EST
In the following snippet, to which model extent should the created EClass be added if mapping f() gets executed? target1 or target2?


mapping f() : EClass @ target1 {
   init {
      result := map g()
   }
}

mapping g() : EClass @ target2 {}

In my opinion, it should first be added to target2, and then moved to target1. However, the current implementation only adds to a model extent on instantiation, so the EClass would stay in target2. I think this should be changed, such that a mapping result is always added to an extent after a mapping has been executed.

Noteworthy, the adding should be done in case of an unspecified model extent. Consider for example a mapping that retrieves its result from a Java blackbox operation. The according result is never added to a model extent (not even to the default extent), and thus not serialized. One nasty workaround is to deepclone() the result, which adds the clone to a model extent.
Comment 1 Christopher Gerking CLA 2016-02-15 09:59:47 EST
Fixed the undoubtful part. If a mapping result is not contained anywhere, it will be attached to the appropriate model extent. Pushed to master.

Commit ID: 9f7811ec25c3501994e61a2cfbecd231026e0328

Adding a dependency to bug 487824, which blocks the desired behavior for abstract syntax blackbox mappings.


The remaining question is: should elements be detatched and reattached if their actual model extent is not the specified one?
Comment 2 Ed Willink CLA 2016-02-15 11:38:25 EST
This is hopefully all addressed by the new QVT 1.3 Section 8.1.3 and in particular 8.1.3.5:

"At the end of the transformation, when out extents are saved, out extents referencing orphans are not well-formed since the orphans are missing; the saved extent is incomplete and difficult to use. Practical implementations
may assist diagnosis of the bad result by rescuing the referenced orphans as members of one or more extents or providing an additional lifeboat output model."

The situation was deliberately defined as not well-formed to avoid imposing any particular rescue approach by an implementation. I slightly favour the lifeboat approach to minimize disruption to the other resources.
Comment 3 Christopher Gerking CLA 2016-02-16 04:14:36 EST
(In reply to Ed Willink from comment #2)
> This is hopefully all addressed by the new QVT 1.3 Section 8.1.3

I'm afraid not, as there is no indication of the circumstances under which the mapping extent applies:

a) Only if the mapping result is newly created by the implicit instantiation section (old buggy behavior).

or

b) Always, i.e., also if the mapping result is fetched from somewhere else. This is the case for mappings with a result initializer inside the init section, or blackbox mappings.

or

c) Only if the mapping result is not yet part of another extent (new current behavior).
Comment 4 Ed Willink CLA 2016-02-16 06:16:08 EST
Surely:

8.1.3.4. "Objects returned as out or inout parameters of a mapping are assigned to an explicit or inferred extent. If no extent can be inferred, typically because two ModelParameters use the same ModelType, the objects are orphans. This is a programming hazard that can be diagnosed by tooling."

answers the one-or-both question. The result is returned from two mappings, it is therefore added to two extents (and since residence in only one extent is possible, it is removed from the first when adding to the second.)

Although the new pseudo-code in 8.1.11.2 omits the establish residence actions, it is difficult to see how the outer residence could be established before the inner residence and so get displaced. Therefore the outer mapping f() starts, the inner mapping g() executes and establishes the target2 residence, and as f() continues , the target1 replacement residence is established.

Perhaps we should expand 8.1.11.2 from:

// instantiation section
if (p3 == null) p3 := object P3{};
if (r1 == null) r1 := object R1{};
if (r2 == null) r2 := object R2{};

to

// instantiation section
if (p3 == null) p3 := object P3{};
if (r1 == null) r1 := object R1{};
if (r2 == null) r2 := object R2{};
if ((extent(p3) <> null) and (extent(p3) <> p3.getModel()) extent(p3).addElement(p3);
if ((extent(r1) <> null) and (extent(r1) <> r1.getModel()) extent(r1).addElement(r1);
if ((extent(r2) <> null) and (extent(r2) <> r2.getModel()) extent(r2).addElement(r2);

where

Element::getModel() is a simple operation returning the model/extent of an element (i.e. EObject::eResource()).

extent(Element) is a pseudo operation that returns the explicit / inferred extent of its argument or null. A bit of OCL should define the inference process on the QVTo AS metamodel.
Comment 5 Sergey Boyko CLA 2016-02-17 02:57:17 EST
(In reply to Christopher Gerking from comment #3)
> 
> I'm afraid not, as there is no indication of the circumstances under which
> the mapping extent applies:
> 
> a) Only if the mapping result is newly created by the implicit instantiation
> section (old buggy behavior).
> 
> or
> 
> b) Always, i.e., also if the mapping result is fetched from somewhere else.
> This is the case for mappings with a result initializer inside the init
> section, or blackbox mappings.
> 
> or
> 
> c) Only if the mapping result is not yet part of another extent (new current
> behavior).

As for QVT 1.2 I think that c) is the most appropriate choice.

As for QVT 1.3 (which introduced Model::addElement()) both b) and c) looks reasonable. However for me choice c) looks more reasonable and I don't see drawbacks for it.
Comment 6 Ed Willink CLA 2016-02-17 04:06:53 EST
I think it has to be b). The user specified @target1. There is nothing in the specification to say this may be ignored. IMHO use of @extent is almost language bloat, I never use it, so if the user has specified it, it is intended.

But note that addElement() specifies removal, with potentially destructive consequences, before (re-)addition. Hence my guards in comment #4 around the addElement().

It may be that some external resources are identifiable as read-only so that an attempt to appropriate one of their elements can be diagnosed.
Comment 7 Christopher Gerking CLA 2016-02-17 05:20:14 EST
(In reply to Ed Willink from comment #6)
> I think it has to be b). The user specified @target1. There is nothing in
> the specification to say this may be ignored.

If the result is already at the root of an extent, I agree that moving to a different extent is obvious. But please note that the result may already have a container element. Will it be decontainerized? In the most weird case, the container element might be part of the very same extent as the specified one:

mapping f() : MyType @ target1 {
   init {
      result := map g();
   }
}

mapping g() : MyType {
   myContainer := ...; -- container is some element inside target1
}

Will the element be decontainerized and moved to the root of its current extent? 


(In reply to Ed Willink from comment #6)
> IMHO use of @extent is almost language
> bloat, I never use it, so if the user has specified it, it is intended.

I agree that @extent is language bloat. It should be the caller's responsibility to attach the mapping result properly.
Comment 8 Ed Willink CLA 2016-02-17 05:33:51 EST
(In reply to Christopher Gerking from comment #7)
> Will the element be decontainerized and moved to the root of its current
> extent? 

If you use addElement() : YES.

If you use the enhanced pseudocode in comment#4 : NO.
Comment 9 Christopher Gerking CLA 2016-02-17 06:51:30 EST
(In reply to Ed Willink from comment #8)
> If you use the enhanced pseudocode in comment#4 : NO.

I see. 

This also answers my question when exactly the result will be attached to an extent. It is sensible to do this as part of the instantiation section. The current implementation does it only after the end section. This is useful because blackbox mappings do not have separate sections, so it is not possible to insert a behavior in between. Therefore the finish of the blackbox mapping is the only possibility to attach the result to an extent. Seems as if we cannot treat blackbox and regular mappings as equal here (same situation as for the creation of the trace record).

What about elements from "in" models? It should be possible to use them as mapping results, without moving them from their input resource. However, if an implicit target extent is inferred, it will lead to exactly this problem. The current implementation circumvents this by checking whether the result is already part of a resource (eResource==null). Is this a sensible behavior? Could it be specified officially?
Comment 10 Ed Willink CLA 2016-02-17 07:41:12 EST
From a QVTo point of view a Blackbox is a single atomic operation so it does not matter in what order the sub-atoms happen; just that the observable effect is consistent.

If an element of an 'in' model is removed from its extent, this seems like an error that can be detected at compile time (and also at run-time). 'in' models are readonly.
Comment 11 Christopher Gerking CLA 2016-02-17 09:40:39 EST
(In reply to Ed Willink from comment #10)
> From a QVTo point of view a Blackbox is a single atomic operation so it does
> not matter in what order the sub-atoms happen; just that the observable
> effect is consistent.

Yes, the implementation just needs to handle these cases separately. No problem.

> If an element of an 'in' model is removed from its extent, this seems like
> an error that can be detected at compile time

How? Such an element may be hidden inside arbitrary collections. I don't think it is feasible to determine statically if the mapping result is an 'in' element or not.

Using an 'in' element as mapping result shouldn't give an error in general. Therefore, since 'in' models are read-only, we need a mechanism to avoid the detachment from the old model extent. Is it possible to specify an 'in' extent using the @input notation? Otherwise, if no such mechanism exists, I'm in favor of the c) semantics, i.e., an element should not be detached from its current model extent, at least not from an 'in' extent. And that is something that should be mentioned in the specification.
Comment 12 Ed Willink CLA 2016-02-18 02:34:12 EST
(In reply to Christopher Gerking from comment #11)
> Using an 'in' element as mapping result shouldn't give an error in general.
> Therefore, since 'in' models are read-only, we need a mechanism to avoid the
> detachment from the old model extent. Is it possible to specify an 'in'
> extent using the @input notation? Otherwise, if no such mechanism exists,
> I'm in favor of the c) semantics, i.e., an element should not be detached
> from its current model extent, at least not from an 'in' extent. And that is
> something that should be mentioned in the specification.

Not all cases of 'in' mutation can be detected at compile time, but the most obvious cases can. An 'in' extent is clearly identified by its use as an 'in' parameter.

IMHO, 

if an @extent is specified, after execution of some 'statement' the object is in the extent specified by that 'statement'. No exceptions.

if an object is in an 'in' extent, the extent is not modified by the 'statements' for which the object is in an 'in' extent. No exceptions. If an 'in' extent is re-used by an 'inout'/'out' extent, a modification of the 'out' extent cannot modify the 'in' extent.

These two contradict only for ill-formed transformations. Ideally execution should fail without modifying the 'in' extent so that in-memory transformation chains can proceed after the error without needing to reload. This could be achieved by using Resource.setTrackingModification(true) but it is an expensive overhead. Alternatively the modest number of mutation points can analysis the 'in'ness of eResource. Much better done by analysis (or by QVTc/QVTr).
Comment 13 Christopher Gerking CLA 2016-02-22 08:54:36 EST
(In reply to Ed Willink from comment #12)
> if an @extent is specified, after execution of some 'statement' the object
> is in the extent specified by that 'statement'. No exceptions.

I agree when it comes to an explicitly specified @extent. But what about mappings without an explicit target extent? I think that an 'in' extent should be valid as an implicit extent and therefore must be considered during the inference mechanism.
Comment 14 Ed Willink CLA 2016-02-22 10:14:21 EST
(In reply to Christopher Gerking from comment #13)
> But what about
> mappings without an explicit target extent? I think that an 'in' extent
> should be valid as an implicit extent and therefore must be considered
> during the inference mechanism.

(In reply to Ed Willink from comment #4)
> extent(Element) is a pseudo operation that returns the explicit / inferred
> extent of its argument or null. A bit of OCL should define the inference
> process on the QVTo AS metamodel.

So we must ensure that the algorithm for the pseudo-operation returns null for an 'in' extent.

Perhaps we rename the pseudo-operation as outExtent(Element).