Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 350982

Summary: Spurious loop counter i
Product: [Modeling] Acceleo Reporter: Ed Willink <ed>
Component: CoreAssignee: Project Inbox <acceleo-inbox>
Status: CLOSED INVALID QA Contact:
Severity: normal    
Priority: P3 CC: laurent.goubet, stephane.begaudeau
Version: 3.0.0   
Target Milestone: ---   
Hardware: PC   
OS: Windows Vista   
Whiteboard:

Description Ed Willink CLA 2011-07-02 04:40:12 EDT
3.1.0

The following template works exactly as I intended

[template public declareOperations(cls : Class)]
[let ops : OrderedSet(Operation) = cls.ownedOperation->asOrderedSet()]
[for (opName : String | ops.name->asOrderedSet()->sortedBy(n : String | n))]
[let op : Operation = ops->any(name = opName)]
public static final EvaluationOperation [cgGetLiteralName(op)/] = new EvaluationOperation(Classes.[cgGetLiteralName(cls)/], [i/], [cgGetImplementationName(op)/]);
[/let]
[/for]
[/let]
[/template]

"i" is an incrementing loop counter, but it is not declared, and when changed to "iii" a compilation error results.

"i" should not be visible.
Comment 1 Stephane Begaudeau CLA 2011-07-04 03:47:57 EDT
That 'i' has been a source of confusion for many users even if it can be very useful when the user needs to iterate on a collection and, at the same time, needs to have a counter.

I don't think that I would create this mechanism like that  if Acceleo was a brand new project. This variable is the only 'magic variable which has not been declared and yet is accessible' in Acceleo and I don't like it very much, I'm not even sure if its usefulness outmatch the confusion it brings to some people. But since this feature has been used by several generators I am pretty sure that it will never be removed for compatibility reasons.
Comment 2 Ed Willink CLA 2011-07-04 04:39:48 EDT
I can appreciate that you now have a compatibility problem, but since this is a clear violation of the specification and probably untenable for nested loops, the default behaviour should change to match the specification. Acceleo cannot claim to be a MOFM2T implementation while this violation is intentional.

For compatibility, you could perhaps introduce warning messages that a helpful variable has automatically been synthesized, and should be rewritten using the

Sequence{1..collection->size()}->iteratorOperation(i : Integer | ... 

idiom.

Perhaps, for Ordered Collection Iterations, OCL might introduce,

  iterator.oclIndex

as an efficient shortform for

  iterationDomain->at(index)

aCollection->forAll(e : Element | f(e, e.oclIndex))

is clearly easier to read and naively more efficient than

Sequence{1..aCollection->size()}->forAll(i : Integer | f(aCollection->at(i), i))

or (for OrderedSet only)

aCollection->forAll(e : Element | f(e, aCollection.indexOf(e)))
Comment 3 Laurent Goubet CLA 2011-07-18 05:30:20 EDT
Ed,

The "i" variable does work for nested loops (of course, if you are within a nested loop, "i" will be the iterator of the inner loop, and will be reset to the iteration value of the outer loop once the inner one ends. "i" will be overriden by explicit "i" variable declaration if the user writes one. It has been added to avoid nested calls to "->at()" in order to find the iteration index of Set/OrderedSet... and to workaround the impossibility to determine that iteration index on Sequence/Bag containing duplicates of the current element.

As for "claim[ing] to be a MOFM2T implementation", there are other violations of the specifications we made in order to simplify Acceleo, and that haven't -yet- been submitted as OMG issues (as I see it, there is no mtt-rtf at the moment...). Not to mention the things we do not (or can not) implement.

I do not know whether "i" is something we'll try and add to the spec, but it has its value since it is often asked by users (who try and have a "counter" variable they increment ... even though OCL cannot have something like "counter = counter + 1") and getting the iteration value through OCL alone is not always possible (duplicates in a Collection).

(In reply to comment #2)
> aCollection->forAll(e : Element | f(e, e.oclIndex))
> 
> is clearly easier to read and naively more efficient than
> 
> Sequence{1..aCollection->size()}->forAll(i : Integer | f(aCollection->at(i),
> i))

Indeed, though to be fair,

aCollection->forAll(e : Element | f(e, i))

would be as readable as

aCollection->forAll(e : Element | f(e, e.oclIndex))

"i" is not accessible in such loops (only in Acceleo "for") because Acceleo cannot change the syntax of the "standard" forAll OCL loop.

All in all, "i" causes no known bug and does not prevent the user to do anything. It will thus stay, at least until we get word from the rtf as to wether this can be added to a future version of the specification or not.