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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 >>>>>>>> [email protected] >>>>>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> Pharo-project mailing list >>>>>>> [email protected] >>>>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> Pharo-project mailing list >>>>>> [email protected] >>>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>> >>>>> >>>>> _______________________________________________ >>>>> Pharo-project mailing list >>>>> [email protected] >>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>>>> >>>> >>>> _______________________________________________ >>>> Pharo-project mailing list >>>> [email protected] >>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >>> >>> >>> _______________________________________________ >>> Pharo-project mailing list >>> [email protected] >>> 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 _______________________________________________ Pharo-project mailing list [email protected] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
