On May 15, 2013, at 4:42 PM, Marcus Denker <marcus.den...@inria.fr> wrote:
>
> On May 15, 2013, at 3:41 PM, Clément Bera <clement.b...@inria.fr> wrote:
>
>> Sorry with the strike and no metro I cannot be at INRIA today. I'm at Marcus
>> right now we will have a look at your slice now :)
>>
>
> So we looked… and there is a problem with
No! We found it! The code showing the error (in the testEnsureTricky) it this:
(String streamContents: [:s |
[
[s nextPutAll: 1 printString. 1/0] ensure: [s nextPutAll: 2
printString]
] on: Error do: [s nextPutAll: 3 printString]
]) asInteger
The problem is that the scope of the second "s" is wrong, it should the block
with the [:s |, but
it is just the block one outer.
And the reason is this:
The ASTClosureAnalyzer, the visitor that analyses if a variable is to be put in
a tempVector or is a copied var, does
this:
visitVariableNode: aVariableNode
"re-lookup the temorary variables..."
| var |
aVariableNode isTemp ifFalse: [^self].
var := scope lookupVar: aVariableNode name.
aVariableNode ocBinding: var.
var isTempVectorTemp ifTrue: [scope
addCopyingTempToAllScopesUpToDefVector: var vectorName].
var isCopying ifTrue: [scope addCopyingTempToAllScopesUpToDefTemp: var].
If it seems a copying temp, it puts copied vars in all scopes until it finds
the definition:
addCopyingTempToAllScopesUpToDefTemp: aVar
(self hasCopyingTempNamed: aVar name) ifFalse: [self addCopyingTemp:
aVar].
tempVars at: aVar name ifPresent: [:v | ^ self].
^ self outerScope addCopyingTempToAllScopesUpToDefTemp: aVar.
Now for the second "s", the visit will call
var := scope lookupVar: aVariableNode name.
again, but this lookup will find the *copy* in the outer scope and returns
this.. so this is clearly wrong.
(but for Opal this is not a problem, as it just compiles a local access to the
copying temp and does not
care that it has a copy...).
Now how to fix it...
Marcus
