|Anonymous | Login||12-07-2021 22:11 UTC|
|Main | My View | View Issues | Change Log | Docs|
|Viewing Issue Advanced Details [ Jump to Notes ]||[ View Simple ] [ Issue History ] [ Print ]|
|ID||Category||Severity||Reproducibility||Date Submitted||Last Update|
|0003448||[Squeak] Compiler||minor||always||04-09-06 22:51||09-07-10 20:03|
|Reporter||nicolas cellier||View Status||public|
|ETA||none||Fixed in Version||Product Version||3.9|
|Summary||0003448: OutOfScopeNotification is caught by ParagraphEditor evaluateSelection|
Thus if you evaluate in a text pane:
Object compile: 'outOfScopeExample
true ifTrue: [| i | i := 1].
^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.
|Steps To Reproduce|
tim argued that above code is equivalent to
| i |
true ifTrue: [i := 1].
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
(0004722 - 294 - 300 - 300 - 300 - 300 - 300)
|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 - 1160 - 1256 - 1256 - 1256 - 1256 - 1256)
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 - 194 - 212 - 212 - 212 - 212 - 212)
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 - 523 - 571 - 699 - 699 - 699 - 699)
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 - 2144 - 2616 - 2616 - 2616 - 2616 - 2616)
Stephane Rollandin reported on squeak-dev "MessageAsTempNode>>scope question" that this one does not work, while it should:
[| a |
a := 1.
(Compiler new evaluate: '1+a' in: thisContext to: nil)
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...
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 - 246 - 274 - 274 - 358 - 358 - 358)
edited on: 05-17-10 07:47
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 - 129 - 141 - 141 - 141 - 141 - 141)
The examples are solved by true closure indeed.
One question remain: why should ParagraphEditor catch OutOfScopeNotification ?
|04-09-06 22:51||nicolas cellier||New Issue|
|04-09-06 22:51||nicolas cellier||File Added: Compiler-Tests-OutOfScope.1.cs|
|04-10-06 09:50||andreas||Note Added: 0004722|
|04-14-06 22:04||nicolas cellier||File Added: Compiler-Patch-OutOfScope.1.cs|
|04-14-06 22:12||nicolas cellier||Note Added: 0004748|
|04-21-06 23:12||nicolas cellier||Note Added: 0004767|
|04-21-06 23:14||nicolas cellier||File Added: Compiler-Patch-OutOfScope.2.cs|
|04-24-06 05:08||nicolas cellier||Note Added: 0004783|
|04-24-06 05:08||nicolas cellier||File Added: Compiler-Patch-OutOfScope.3.cs|
|04-12-07 16:35||nicolas cellier||Note Added: 0010532|
|05-17-10 07:45||FrankShearar||Note Added: 0013780|
|05-17-10 07:47||FrankShearar||Note Edited: 0013780|
|09-07-10 20:03||nicolas cellier||Note Added: 0013868|
| Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
69 total queries executed.|
41 unique queries executed.