Mantis - Squeak
Viewing Issue Advanced Details
3448 Compiler minor always 04-09-06 22:51 09-07-10 20:03
nicolas cellier  
 
normal  
new 3.9  
open  
none    
none  
0003448: OutOfScopeNotification is caught by ParagraphEditor evaluateSelection
Thus if you evaluate in a text pane:

  Object compile: 'outOfScopeExample
    true ifTrue: [| i | i := 1].
    ^i'.
  ^Object new perform: #outOfScopeExample

you will get 1 as result.
but if you put same code in a file and file it in via FileBrowser menu for example, you get a SyntaxError.
Also you cannot modify the method in a browser, load it from Monticello, etc...
That is an unconsistent behavior.
tim argued that above code is equivalent to
  outOfScopeExample
    | i |
    true ifTrue: [i := 1].
    ^i
because there is no proper closure in default compiler.
That is correct, but then does this suggest the OutOfScopeNotification should be removed everywhere ?
Or shouldn't we better enforce the notification everywhere ?

If we want to enforce the notification, we must remove the exception handling from ParagraphEditor. But we first have to change the method MessageAsTempNode>>#scope to return 0 or receiver scope, instead of -1. This will cure the inspector and debugger contextual evaluations.

But then, there might be other problems in script evaluation i am not aware of, that did maybe justify the exception handling... so my help stops here.
 Compiler-Tests-OutOfScope.1.cs [^] (1,767 bytes) 04-09-06 22:51
 Compiler-Patch-OutOfScope.1.cs [^] (5,072 bytes) 04-14-06 22:04
 Compiler-Patch-OutOfScope.2.cs [^] (5,073 bytes) 04-21-06 23:14
 Compiler-Patch-OutOfScope.3.cs [^] (5,121 bytes) 04-24-06 05:08

Notes
(0004722)
andreas   
04-10-06 09:50   
I think the real issue is that it's perfectly fine for ParagraphEditor to catch OutOfScope during *compilation* but not when *evaluating* the code. In other words, Compiler>>evaluate: should really be broken into two parts, compilation (which handles OutOfScope) and evaluation (which doesn't).
(0004748)
nicolas cellier   
04-14-06 22:12   
Andreas, very nice way of not breaking anything.
In this case, the change should be easy to make...
Well, except these details:
- #tallySelection call a non implemented method (at least not implemented in 3.9a 7020 and old compiler)
- #compileSelectionFor:in: (which is called by #debugIt) explicitely call Compiler instead of evalutaorClass new

So, two minor fixes are also required.
Also note that debugIt and tallySelection do not seem to write in the change log...

A small thing remain: eventually change cannot be replayed from a change list browser, but that is also the case for contextual DoIt, and maybe we can forget about it.

There is also another thing i do not like: FakeClassPool>>#adopt:
Changing a global var for passing information underground is not something very pure!
It is not tailored for multi-process for example... we'd rather not follow the path of VW making multi-process UI, or we'll bump into this one.

The patch i propose is not very light: it adds two methods to compiler (to separate generate... and execute...), and should be ported to new compiler also. Maybe code could be better factored, but it works...
(0004767)
nicolas cellier   
04-21-06 23:12   
Oops! my first patch has a typo bug (context instead of aContext).
That prevent some contextual evaluation to work...

So, throw away Compiler-Patch-OutOfScope.1.cs and rather take version 2.
(0004783)
nicolas cellier   
04-24-06 05:08   
as stated in http://bugs.impara.de/view.php?id=3498, [^]
there is another problem in ParagraphEditor>>compileSelectionFor:in:
it does not set method selector to #DoIt, and then this make MessageTally fail.

Note that this method was used only by debugIt menu, and that this menu work because it really install the method in the MethodDictionary with the side effect of setting method selector and class...

But since i want to use compileSelectionFor:in: in another place, i need a patch.
So i provide patch version 3...
(0010532)
nicolas cellier   
04-12-07 16:35   
Stephane Rollandin reported on squeak-dev "MessageAsTempNode>>scope question" that this one does not work, while it should:
[| a |
    a := 1.
         Transcript show:
        (Compiler new evaluate: '1+a' in: thisContext to: nil)
] fork

Compiler-Patch-OutOfScope.2.cs does not fix this, neither does .3.cs.
They are cosmetic changes just in the hope to not break anything...
But they only cure one side effect, not the main problem.
So i went back to first proposition in additionnal information.

MY CONCLUSIONS ARE:

1) What is the meaning of the OutOfScopeNotification? to prevent access of Block local variable outside the block.
2) a temporary in the scope of thisContext is not OutOfScope,
   however, it will raise an OutOfScopeNotification
   because MessageAsTempNode>>scope does return -1.
3) an error handling in ParagraphEditor suppress the OutOfScopeNotification in all cases (for thisContext scope and for true outOfScope cases), but only in interactive mode from within a text pane, not in batch or fork mode.
4) contextual variables might be used outside interactive text pane mode, as shown by Stephane example (also imagine a workspace contents filed in in batch)
5) IT IS GOOD TO ALWAYS ENFORCE RULE 1),
   even if byte code implementation enables such access.
   (and we shouldn't care of implementation, just formal definition of language)
   I do not see valuable exceptions when we should allow local variable access out of scope.
6) Putting a Compiler error handling in ParagraphEditor would make sense if error handling is to interact with user
   (because we know compiler is used in interactive mode).
   Here, it is the contrary, we blindly accept when in interactive mode and refuse to compile in batch mode...

ACTION REQUEST:

Given 1) 2) 3) and 4) indeed MessageAsTempNode>>scope should return 0.

Anyone has an objection ? (case when a MessageAsTempNode could be out of scope?)

Given 5) and 6), we should remove handling of the notification in ParagraphEditor.

Anyone has an objection ? (Good reason to access out of scope)
Anyone to check if any implication in Etoys or something?
(0013780)
FrankShearar   
05-17-10 07:45   
With the fix for 0007532, this now returns nil. If you browse to Object>>outOfScopeExample in trunk, you'll see that the out of scope i is highlighted in red. If you edit (say by adding a trailing space) and recompile, you're told "out of scope".

(0013868)
nicolas cellier   
09-07-10 20:03   
The examples are solved by true closure indeed.

One question remain: why should ParagraphEditor catch OutOfScopeNotification ?