Naturally in the Pharo Inbox: Name: Tests-lr.122 Author: lr Time: 8 March 2010, 3:12:26 pm UUID: faa9eb31-f84e-4e49-916e-991115a6f2da Ancestors: Tests-StephaneDucasse.121 - added tests for the compiler to test shadowing of self, super, thisContext, true, false and nil On 8 March 2010 15:13, Lukas Renggli <renggli@gmail.com> wrote:
Here are 3 additional tests that verify that these reserved variables are not shadowed.
Please integrate :-)
Lukas
On 8 March 2010 15:01, Lukas Renggli <renggli@gmail.com> wrote:
Actually this is already fixed by the previous fix. Jorge and I are going to write some new tests.
Lukas
On 8 March 2010 14:50, Lukas Renggli <renggli@gmail.com> wrote:
Erwann just noticed another kind of serious shadowing bug.
The closure compiler allows to declare methods like the following:
foo: self   | super thisContext true false nil |   ^ super := thisContext := true := false := nil := self
These argument and temporary names should all be disallowed as all other Smalltalk compiler do (and also the pre closure one in Pharo did). Using the implicit variables as argument or temp names leads to absolutely unmaintainable code.
Lukas
On 5 March 2010 12:16, Lukas Renggli <renggli@gmail.com> wrote:
The problem is that some ancestor versions disappeared, what makes it impossible to merge. We've manually integrated the delta into:
Name: Tests-lr.79 Author: lr Time: 5 March 2010, 12:16:05 pm UUID: 99f8b58a-0923-4264-9ea6-58fec6cf4d18 Ancestors: Tests-MichaelRueger.78
- merged Jorges Code
Lukas
On 5 March 2010 11:25, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
Jorge
could you publish the tests for 1.1 because I could not find my way 11 conflicts for 1.1?
Stef
Hi Adrian,
Sorry for the delay, too many things today.
Ok, I added 17 test to Tests-Compiler which model the cases of shadowing, at least the ones I could come up with. I also modified the Encoder>>bindTemp: in order to make it a little bit more intention revealing.
My changes are in:
           Compiler-JorgeRessia.144            Tests-JorgeRessia.46
Some comments:
- In order to test the not interactive mode I had to mock the Transcript. I could not find a better solution, if anybody has a better one just let me know I'll change it. - There is a special case which is that when you are NOT in interactive mode if the shadowed variable is a temp then the syntax error is triggered as in interactive mode. This is the behavior implemented before I did the changes. Is that what we want? - I added this test to Tests-Compiler package, I would have preferred to add them to Compiler-Tests but there was nothing there.
Cheers,
Jorge
On Thu, Mar 4, 2010 at 8:45 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
Hi Jorge,
Just let me know when you think the code is ok for 1.0 or when you have something different that I should integrate. Thanks!
Cheers, Adrian
On Mar 4, 2010, at 09:37 , Jorge Ressia wrote:
Hi Adrian,
Yep, I have the same problem as you. My idea was to make it work as before without completely changing it. Basically because I did not have the test support to validate my changes. Anyhow, what I propose is to write a bunch of tests for every case that we know and then if new cases show up add them to the test suite. And then play with the code.
I will work on that today, is that ok?
For the explanation of what is going on is that now node can be an instance variable node, then the behavior that was there was not appropriate in the sense that instance variable nodes do not understand #scope. However, why this validation "(node isTemp or: [requestor interactive])" was there in the first place is not that clear to me, but it's been there since 1999.
I'll try to build the tests and come up with a better solution.
Cheers,
Jorge
On Thu, Mar 4, 2010 at 9:14 AM, Adrian Lienhard <adi@netstyle.ch> wrote:
Jorge,
I looked at the code to integrate in 1.0. And I really have troubles understanding it (even after drawing a boolean table for the different combinations of or: ifTrue: and: ifFalse:). Is it correct that the warning is shown when node isTemp is false and requestor interactive is true? Is the comment still accurate?
This is the code:
"When non-interactive raise the error only if its a duplicate" (node isTemp or: [requestor interactive]) Â Â Â Â ifTrue:[ ((node isTemp) and: [node scope <= 0] ) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ifFalse: [^self notify:'Name is already defined' ]]
Cheers, Adrian
On Mar 3, 2010, at 20:47 , Stéphane Ducasse wrote:
11243 -----
- Issue 2102: Fix the fact that local temp can shadow silently instance var --- fixed    Problem fixed: We cannot have twice the same block arg in a method
   [:each | each ...]    [:each | each ...]
THANKS jorge :) Do you like good bio beer?
Stef
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- Lukas Renggli http://www.lukas-renggli.ch
-- Lukas Renggli http://www.lukas-renggli.ch
-- Lukas Renggli http://www.lukas-renggli.ch
-- Lukas Renggli http://www.lukas-renggli.ch
-- Lukas Renggli http://www.lukas-renggli.ch