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