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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Eliot Miranda-2


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







_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

Encoder-encodeVariablesourceRangeifUnknown.st (992 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Levente Uzonyi-2
On Sun, 16 May 2010, Eliot Miranda wrote:

> 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

I would also expect the following to compile if temporaries can
shadow instance variables:

Foo >> #foo

  | b |
  a := 1.
  b := [ :a | a ].
  ^b value: a.

Where a is an instance variable of Foo. But I get:

  ^b value:  out of scope ->a.

Which doesn't make much sense IMHO.


Levente

>
>
>> 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
>>>
>>>
>>>
>>>
>>
>>
>

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

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

Stéphane Ducasse
In reply to this post by Eliot Miranda-2
Thanks eliot

http://code.google.com/p/pharo/issues/detail?id=2436

In 1.1

| a |
a := 4.
b := [ :a | a ].
c := b value: a.

we cannot shadow a

Stef


On May 17, 2010, at 2:47 AM, Eliot Miranda wrote:

>
>
> 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


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project