Re: [squeak-dev] Re: Problem with variable capture in blocks or ? #7532

> On Sun, May 16, 2010 at 3:32 PM, Andreas Raab <[hidden email]> wrote:
>
>> Hi Ken -
>>
>> Very interesting. Looks like an issue with OutOfScopeNotification. The
>> problem appears to be that this code:
>>
>>        "..." [ :a | a ].
>>        "..." a.
>>
>> generates an OutOfScopeNotification that is being suppressed in workspaces
>> (I'm not exactly sure why that is). This is of course is interesting because
>> of the highly unusual situation of a temp shadowing a global instead of
>> another temp / ivar. For example, this wouldn't compile:
>>
>>  | a |
>>  [ :a | a ].
>>
>> But stranglely, this does:
>>
>>  [ :Object | Object ].
>>
>> so your example code is roughly equivalent to running:
>>
>>        b := [ :Object | Object ].
>>        c := b value: Object.
>>
>> When you run this line by line it works 'as expected' and when you run it
>> as a single doIt you get the result of c being a "[closure] in
>> UndefinedObject>>DoIt" due to the OutOfScopeNotification.
>>
>> Hope this explains the issue - as for how to fix it, I have no clue :-)
>>
>
> Yes, the bug is that Encoder>>encodeVariable:sourceRange:ifUnknown: doesn't
> continue from a caught OutOfScopeNotification by answering the ifUnknown:
> action.  Instead it returns the out-of-scope block temp.  This is a rather
> serious compiler bug that I'd left unfixed because it only bites in
> workspaces (apologies) and I didn't have a small example to analyse before
> (thanks Ken!).  The code generated is actually equivalent to
>
>       b := [ :a | a ].
>       c := b value: <temp 0>.
>
> Since there aren't any temps, temp 0 is top of stack, which just happens to
> be the closure just created by the preceding bytecode.  e.g. look at
> bytecode 49 in the following:
>
> a := 4. b := [ :a | a ]. c := b value: a. { a. b. c } thisContext method
> symbolic
>
> The fix is simple.  When an OutOfScopeNotification is caught the Encoder
> should still answer what ever it should for an out of scope variable.   In
> Encoder>>encodeVariable:sourceRange:ifUnknown: the statements
>
> (varNode isTemp and: [varNode scope < 0]) ifTrue: [
> OutOfScopeNotification signal ifFalse: [ ^self notify: 'out of scope'].
> ].
> ^ varNode
>
> should read
>
> (varNode isTemp and: [varNode scope < 0]) ifTrue:
> [^OutOfScopeNotification signal
> ifTrue: [action value]
> ifFalse: [self notify: 'out of scope']].
> ^varNode
>
> Find attached:
>
> !Encoder methodsFor: 'encoding' stamp: 'eem 5/16/2010 17:33'!
> encodeVariable: name sourceRange: range ifUnknown: action
> | varNode |
> varNode := scopeTable
> at: name
> ifAbsent:
> [(self lookupInPools: name
> ifFound: [:assoc | varNode := self global: assoc name: name])
> ifTrue: [varNode]
> ifFalse: [^action value]].
> range ifNotNil:
> [name first canBeGlobalVarInitial ifTrue:
> [globalSourceRanges addLast: { name. range. false }]].
>
> (varNode isTemp and: [varNode scope < 0]) ifTrue:
> [^OutOfScopeNotification signal
> ifTrue: [action value]
> ifFalse: [self notify: 'out of scope']].
> ^varNode! !
>
> P.S. this is probably applicable to any Squeak bytecode compiler (including
> eToys).  The only change in my closure compiler from Andreas' 2003 version
> was changing "name first isUppercase" to "name first canBeGlobalVarInitial".
>
>
> P.P.S.  Again Ken, thanks for a comprehensible example.  It always bit me in
> huge doits I was using to analyse the entire system's compiled methods, and
> invariably crashed the VM.  I never took the time to isolate the bug, I just
> fixed the doit and continued.  Turns out to be very simple.
>
> best
> Eliot

>
>
>> Cheers,
>>  - Andreas
>>
>>
>>
>> On 5/16/2010 12:35 PM, Ken Causey wrote:
>>
>>> Well, it's nothing new but this one has stumped me:
>>>
>>> http://bugs.squeak.org/view.php?id=7532
>>>
>>> Initially I (and Frank) thought the reporter was mistaken until we
>>> understood that the problem shows up when you execute the main code all
>>> in one do-it.  I've since modified the original report to make this
>>> clearer.
>>>
>>> So here it is:
>>>
>>> a := 4.
>>> b := [ :a | a ].
>>> c := b value: a.
>>>
>>> If you SELECT ALL OF THIS AND EXECUTE IT ALL AT ONE TIME (crucial
>>> detail). The result is that a is 4, b is a BlockClosure, and c is a
>>> BlockClosure not 4 as expected. Execute each statement separately and c
>>> is 4.
>>>
>>> Alternately, from a suggestion from jmckeon, if you specify a different
>>> symbol for the block argument:
>>>
>>> a := 4.
>>> b := [ :d | d ].
>>> c := b value: a.
>>>
>>> when executed all at one time works as you would expect: a is 4, b is a
>>> BlockClosure, and c is 4. (and d is nil)
>>>
>>> So what's up?
>>>
>>> Ken
>>>
>>>
>>>
>>>
>>
>>
>

>
>
> On Sun, May 16, 2010 at 3:32 PM, Andreas Raab <[hidden email]> wrote:
> Hi Ken -
>
> Very interesting. Looks like an issue with OutOfScopeNotification. The problem appears to be that this code:
>
>        "..." [ :a | a ].
>        "..." a.
>
> generates an OutOfScopeNotification that is being suppressed in workspaces (I'm not exactly sure why that is). This is of course is interesting because of the highly unusual situation of a temp shadowing a global instead of another temp / ivar. For example, this wouldn't compile:
>
>  | a |
>  [ :a | a ].
>
> But stranglely, this does:
>
>  [ :Object | Object ].
>
> so your example code is roughly equivalent to running:
>
>        b := [ :Object | Object ].
>        c := b value: Object.
>
> When you run this line by line it works 'as expected' and when you run it as a single doIt you get the result of c being a "[closure] in UndefinedObject>>DoIt" due to the OutOfScopeNotification.
>
> Hope this explains the issue - as for how to fix it, I have no clue :-)
>
> Yes, the bug is that Encoder>>encodeVariable:sourceRange:ifUnknown: doesn't continue from a caught OutOfScopeNotification by answering the ifUnknown: action.  Instead it returns the out-of-scope block temp.  This is a rather serious compiler bug that I'd left unfixed because it only bites in workspaces (apologies) and I didn't have a small example to analyse before (thanks Ken!).  The code generated is actually equivalent to
>
>        b := [ :a | a ].
>        c := b value: <temp 0>.
>
> Since there aren't any temps, temp 0 is top of stack, which just happens to be the closure just created by the preceding bytecode.  e.g. look at bytecode 49 in the following:
>
> a := 4. b := [ :a | a ]. c := b value: a. { a. b. c } thisContext method symbolic
>
> The fix is simple.  When an OutOfScopeNotification is caught the Encoder should still answer what ever it should for an out of scope variable.   In Encoder>>encodeVariable:sourceRange:ifUnknown: the statements
>
> (varNode isTemp and: [varNode scope < 0]) ifTrue: [
> OutOfScopeNotification signal ifFalse: [ ^self notify: 'out of scope'].
> ].
> ^ varNode
>
> should read
>
> (varNode isTemp and: [varNode scope < 0]) ifTrue:
> [^OutOfScopeNotification signal
> ifTrue: [action value]
> ifFalse: [self notify: 'out of scope']].
> ^varNode
>
> Find attached:
>
> !Encoder methodsFor: 'encoding' stamp: 'eem 5/16/2010 17:33'!
> encodeVariable: name sourceRange: range ifUnknown: action
> | varNode |
> varNode := scopeTable
> at: name
> ifAbsent:
> [(self lookupInPools: name
> ifFound: [:assoc | varNode := self global: assoc name: name])
> ifTrue: [varNode]
> ifFalse: [^action value]].
> range ifNotNil:
> [name first canBeGlobalVarInitial ifTrue:
> [globalSourceRanges addLast: { name. range. false }]].
>
> (varNode isTemp and: [varNode scope < 0]) ifTrue:
> [^OutOfScopeNotification signal
> ifTrue: [action value]
> ifFalse: [self notify: 'out of scope']].
> ^varNode! !
>
> P.S. this is probably applicable to any Squeak bytecode compiler (including eToys).  The only change in my closure compiler from Andreas' 2003 version was changing "name first isUppercase" to "name first canBeGlobalVarInitial".
>
>
> P.P.S.  Again Ken, thanks for a comprehensible example.  It always bit me in huge doits I was using to analyse the entire system's compiled methods, and invariably crashed the VM.  I never took the time to isolate the bug, I just fixed the doit and continued.  Turns out to be very simple.
>
> best
> Eliot
>
>
> Cheers,
>  - Andreas
>
>
>
> On 5/16/2010 12:35 PM, Ken Causey wrote:
> Well, it's nothing new but this one has stumped me:
>
> http://bugs.squeak.org/view.php?id=7532
>
> Initially I (and Frank) thought the reporter was mistaken until we
> understood that the problem shows up when you execute the main code all
> in one do-it.  I've since modified the original report to make this
> clearer.
>
> So here it is:
>
> a := 4.
> b := [ :a | a ].
> c := b value: a.
>
> If you SELECT ALL OF THIS AND EXECUTE IT ALL AT ONE TIME (crucial
> detail). The result is that a is 4, b is a BlockClosure, and c is a
> BlockClosure not 4 as expected. Execute each statement separately and c
> is 4.
>
> Alternately, from a suggestion from jmckeon, if you specify a different
> symbol for the block argument:
>
> a := 4.
> b := [ :d | d ].
> c := b value: a.
>
> when executed all at one time works as you would expect: a is 4, b is a
> BlockClosure, and c is 4. (and d is nil)
>
> So what's up?
>
> Ken
>
>
>
>
>
>
> <Encoder-encodeVariablesourceRangeifUnknown.st>_______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project