OutOfScopeNotification
Locked
 
|
About http://bugs.squeak.org/view.php?id=3448, the visible
consequences are fixed by introduction of closure, but... Can someone explain why should ParagraphEditor & al catch OutOfScopeNotification at all ? Nicolas |
|
|
On Tue, Sep 7, 2010 at 1:07 PM, Nicolas Cellier <[hidden email]> wrote: About http://bugs.squeak.org/view.php?id=3448, the visible Looking at the Mantis case I'd say it was obsolete and ideally we would nuke the entire class and its uses, but we need to change the part of Encoder>>#encodeVariable:sourceRange:ifUnknown: that says
(varNode isTemp and: [varNode scope < 0]) ifTrue: [^OutOfScopeNotification signal
ifTrue: [action value] ifFalse: [self notify: 'out of scope']].
to "do the right thing". This isn't the right thing: (varNode isTemp and: [varNode scope < 0]) ifTrue:
[^action value]. Because for an out-of-scope temp this will cause a raise of an UndeclaredVariableWarning, which is wrong, which if uncaught will result in a notification of "Undeclared", which is misleading.
Instead one fix might be to change Encoder>>encodeVariable: name to something like Encoder>>encodeVariable: name ^self encodeVariable: name
sourceRange: nil ifOutOfScope:
[:nodeOrNil| nodeOrNil
ifNil: [self undeclared: name]
ifNotNil: [self assert: (nodeOrNil isTemp and: [nodeOrNil scope < 0]).
requestor notify: (nodeOrNil isArg ifTrue: ['block arg'] ifFalse: ['temp']),
' is out of scope']]
rename Encoder>>#encodeVariable:sourceRange:ifUnknown: to Encoder>>#encodeVariable:sourceRange:ifOutOfScope: and have it pass in either nil or the out-of-scope temp.
What would you do? best, Eliot
|
|
|
It depends how we tolerate shadowing...
For example, if we allow a temp shadowing an ivar, then Compiler shall distinguish the cases : - variable is used out of scope -> notify: if interactive - variable has another external scope -> just warn: In such case, if we refactor the class and remove the ivar, we should better create a binding in Undeclared rather than raising a compilation Error. If we do not tolerate shadowing, then maybe we can ignore the edge case and always bark. But there is the problem of Workspace. In a Workspace, we should better tolerate temp shadowing workspace variables. In effect, the compiler does not bark in following case even if we remove OutOfScopeNotification handling from ParagraphEditor: a := 4. a > 0 ifTrue: [^a] ifFalse: [| a | a := 1. ^a]. However, this one: a := 4. a > 0 ifFalse: [| a | a := 1. ^a]. ^a will notify an ' out of scope ->' A bit weird... Workspace concept can also be used for example for saving complex Object graphs with messages reconstructing the graph and with variables for handling cycles. If we want to store blocks (a sortBlock for example) with local temps without caring too much of workspace variable name conflicts, then shadowing is attractive. Next question is: can Encoder deal with outer scope of a shadowed ? Nicolas 2010/9/8 Eliot Miranda <[hidden email]>: > > > On Tue, Sep 7, 2010 at 1:07 PM, Nicolas Cellier > <[hidden email]> wrote: >> >> About http://bugs.squeak.org/view.php?id=3448, the visible >> consequences are fixed by introduction of closure, but... >> >> Can someone explain why should ParagraphEditor & al catch >> OutOfScopeNotification at all ? > > Looking at the Mantis case I'd say it was obsolete and ideally we would nuke > the entire class and its uses, but we need to change the part of > Encoder>>#encodeVariable:sourceRange:ifUnknown: that says > (varNode isTemp and: [varNode scope < 0]) ifTrue: > [^OutOfScopeNotification signal > ifTrue: [action value] > ifFalse: [self notify: 'out of scope']]. > to "do the right thing". This isn't the right thing: > (varNode isTemp and: [varNode scope < 0]) ifTrue: > [^action value]. > Because for an out-of-scope temp this will cause a raise of an > UndeclaredVariableWarning, which is wrong, which if uncaught will result in > a notification of "Undeclared", which is misleading. > Instead one fix might be to change Encoder>>encodeVariable: name to > something like > Encoder>>encodeVariable: name > ^self encodeVariable: name > sourceRange: nil > ifOutOfScope: > [:nodeOrNil| > nodeOrNil > ifNil: [self undeclared: name] > ifNotNil: [self assert: (nodeOrNil isTemp and: [nodeOrNil scope < 0]). > requestor notify: (nodeOrNil isArg ifTrue: ['block arg'] ifFalse: > ['temp']), > ' is out of scope']] > rename Encoder>>#encodeVariable:sourceRange:ifUnknown: > to Encoder>>#encodeVariable:sourceRange:ifOutOfScope: and have it pass in > either nil or the out-of-scope temp. > What would you do? > best, > Eliot > >> >> Nicolas >> > > > > > |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |