Bugzilla – Full Text Bug Listing
|
Description
Dennis Hendriks
Created attachment 178787 [details]
test.qvto: Correct behavior, result assigned in init block
Created attachment 178788 [details]
test2.qvto: Weird behavior, result assigned outside of init block
Created attachment 220815 [details] Patch for error on non-incremental result assignments outside init-section Assignments to the 'result' variable outside the init section are likely to be enabled by the patch proposed for bug 388325 (to support incremental assignments to the 'result' variable). Hence, the patch attached here prevents default assignments to the 'result' variable outside the init section by raising a proper compilation error. The error indication should be possible to be carried out safely, since the prevented assignments were effectless up to now either way. Created attachment 222456 [details]
Patch for error on non-incremental result assignments outside init-section (updated copyright and contributors)
Patch for error on non-incremental result assignments outside init-section (updated copyright and contributors)
Created attachment 222463 [details]
JUnit test patch
Created attachment 222562 [details] Updated patch Also fixes bug 302594, bug 310991 and bug 392157. Created attachment 223667 [details]
JUnit test patch
Hi Christopher, +1. Trivial comments: - QvtOperationalVisitorCS has unproper copyright header. - Again: when extendedClassifier instanceof EClass then always extendedClassifier != null. Comment on attachment 222562 [details]
Updated patch
Patch set to obsolete, because it didn't provide a universal solution. Considering the eContainer is not enough, since assignments can be nested arbitrarily deep (not necessarily referring to MappingInitCS as container).
For a universal solution, I propose to "loop through" an additional boolean parameter value named e.g. 'resultAssignmentAllowed'. This one should only be true during the init section evaluation. If it is false and one attempts a non-incremental assignment, a compile error should be raised.
- The copyright header already looked improper before I changed anything, just wasn't sure about fixing it. - The part with the null comparison is not used anymore. Hi Christopher, I'm being worried of making messier the already messy bunch of bugs we are reviewing/adding comments. Dont include more patches unless, there is a clear -1. Let's try to have the big one in, and any future change can be made once the big patch is pushed to master, well resolved bugs are closed, etc Cheers, Adolfo. I agree that the proposed change should be postponed... Correct behavior for this request is to prohibit assignment of 'result' outside init{} section.
The reason is that such assignment breaks fundamental QVT invariant - relation trace is populated at the end of an implicit "instantiation section" (which is executed right after init{} section if this one is declared). From that point the relation is considered to hold and the trace data becomes available.
See QVT specification section "8.2.1.15 MappingOperation" topic "Executing a mapping operation".
Created attachment 229195 [details] Prohibits re-assignment of 'result' outside init{} mapping section This patch also supports incremental assignment ('+=' operator) for mutable QVTo types - List and Dict. Also it remediates problem with saving of incomplete Trace instance mentioned in https://bugs.eclipse.org/bugs/show_bug.cgi?id=388325#c19 Abbreviated .qvto snippet: modeltype Ecore uses ecore('http://www.eclipse.org/emf/2002/Ecore'); transformation NewTransformation(); main() { var v1 : Set(EClass) = object EClass{}.map mapToSet(); assert fatal (v1->size() = 2); var v2 : List(EClass) = object EClass{}.map mapToList(); assert fatal (v2->size() = 2); v2->add(object EClass{}); var resolved2 : List(EClass) = resolveoneIn(EClass::mapToList, List(EClass)); assert fatal (resolved2->size() = 3); } mapping EClass::mapToSet() : Set(EClass) { init { result := object EClass{}->asSet(); result += object EClass{}; } } mapping EClass::mapToList() : List(EClass) { init { result := object EClass{}->asList(); } result += object EClass{}; } I'm not sure if this solution is very comprehensible. Although most collections are immutable, there is still support for "simulated" incremental assignments inside the init section. If I understand your latest patch correctly, all other sections should not support this simulation of incremental assignments, just because the trace record has already been stored before. Can't we update the trace whenever 'result' is subject to an incremental assignment? Otherwise, +1 for your patch which at least ensures consistency. (In reply to comment #15) > I'm not sure if this solution is very comprehensible. > > Although most collections are immutable, there is still support for > "simulated" incremental assignments inside the init section. If I understand > your latest patch correctly, all other sections should not support this > simulation of incremental assignments, just because the trace record has > already been stored before. > I'll try to describe the point once again: 1. Re-assignment of 'result' variable is not possible outside the init{} section by any means (https://bugs.eclipse.org/bugs/show_bug.cgi?id=325192#c13). 2. For mutable collections (that is for List) statement 'result +=' is shorthand for 'result->add()' method invocation. 3. For immutable collections (that is for Set, Sequence, etc.) statement 'result +=' is shorthand for a) creation of new collection which consists of original plus new element and b) assignment of this new collection to 'result'. As you see option 3b) complies with option 1 only inside the init{} section. What you call "simulated incremental assignments inside the init section" is in fact just repetition of assignment. Code snippet: mapping EClass::mapToSet() : Set(EClass) { init { result := object EClass{}->asSet(); result += object EClass{}; } } Is equal to: mapping EClass::mapToSet() : Set(EClass) { init { result := object EClass{}->asSet(); result := result->including(object EClass{}); } } As you might expect the number of re-assignments to 'result' in init{} section is not limited. But 'result' can't be re-assigned after the end of init{} section. Pushed to master for Kepler M7. Commit ID: 688d5773fb3bb2714459e9f7fe4b17c471581bac New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/187933 New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/187965 Gerrit change https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/187965 was merged to [master]. Commit: http://git.eclipse.org/c/mmt/org.eclipse.qvto.git/commit/?id=cfdd8091913291545f19d5f3e717a35f98d97371 |