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
