[update 1.1] #11243

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[update 1.1] #11243

Stéphane Ducasse
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
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Adrian Lienhard
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
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

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

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Stéphane Ducasse

On Mar 4, 2010, at 9:37 AM, 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?

excellent.

> 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.

You what I would love to spend 2/3 weeks just coding in the new compiler and making sure
that it has the right abstractions/messages to deal with issue like that.

I looks to me that the compiler is pre object design time from time to time

>
> Cheers,
>
> Jorge
>
> On Thu, Mar 4, 2010 at 9:14 AM, Adrian Lienhard <[hidden email]> 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
>>> [hidden email]
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Lukas Renggli
>> I'll try to build the tests and come up with a better solution.
>
> You what I would love to spend 2/3 weeks just coding in the new compiler and making sure
> that it has the right abstractions/messages to deal with issue like that.

The problem is that the New Compiler is not really in a better state
than the old compiler. Of course it has more objects and is much
easier to extend, but all in all it is a mess too. This is mostly due
to the fact that the system below it changed quite a bit in the past
years (pragmas, properties, closures, primitives, new bytecodes, ...)
and that the New Compiler had to be patched and changed quite heavily
to accommodate these new requirements that it was not designed for.
Maintaining and fixing the New Compiler got so extremely expensive
that it is questionable if this is still a viable platform? Ask Jorge
and Marcus.

Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Jorge Ressia
Yes, the New compiler is not better. However, I think the New Compiler
is the right direction to pursue.

After fighting with it for some months to make the Closures work in it
I can tell you that I learn a great deal from it, painfully :) .
But I now have a pretty good idea of what it is required to have a
modularized compiler with more suitable abstraction for this
particular domain.
I implemented many tests and I have to implement a lot more. Then, I
believe that by expressing ourselves through these tests we will be
able to modify the compiler to make it look the way we want.

At least that is my plan with the New compiler.

Cheers,

Jorge

On Thu, Mar 4, 2010 at 9:53 AM, Lukas Renggli <[hidden email]> wrote:

>>> I'll try to build the tests and come up with a better solution.
>>
>> You what I would love to spend 2/3 weeks just coding in the new compiler and making sure
>> that it has the right abstractions/messages to deal with issue like that.
>
> The problem is that the New Compiler is not really in a better state
> than the old compiler. Of course it has more objects and is much
> easier to extend, but all in all it is a mess too. This is mostly due
> to the fact that the system below it changed quite a bit in the past
> years (pragmas, properties, closures, primitives, new bytecodes, ...)
> and that the New Compiler had to be patched and changed quite heavily
> to accommodate these new requirements that it was not designed for.
> Maintaining and fixing the New Compiler got so extremely expensive
> that it is questionable if this is still a viable platform? Ask Jorge
> and Marcus.
>
> Lukas
>
> --
> Lukas Renggli
> http://www.lukas-renggli.ch
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Marcus Denker-4
In reply to this post by Lukas Renggli

On Mar 4, 2010, at 9:53 AM, Lukas Renggli wrote:

>>> I'll try to build the tests and come up with a better solution.
>>
>> You what I would love to spend 2/3 weeks just coding in the new compiler and making sure
>> that it has the right abstractions/messages to deal with issue like that.
>
> The problem is that the New Compiler is not really in a better state
> than the old compiler. Of course it has more objects and is much
> easier to extend, but all in all it is a mess too. This is mostly due
> to the fact that the system below it changed quite a bit in the past
> years (pragmas, properties, closures, primitives, new bytecodes, ...)
> and that the New Compiler had to be patched and changed quite heavily
> to accommodate these new requirements that it was not designed for.
> Maintaining and fixing the New Compiler got so extremely expensive
> that it is questionable if this is still a viable platform? Ask Jorge
> and Marcus.

It needs work.... the problem is that I did not have any time and energy to work
on it. (traveling/moving and learning new languages are nice, but two time in one year
was too much...).

But I *definitly* need the infrastructure... in some form. I need to get Reflectivity to
work again, it's needed for at least Jorge's work and JB's work... and for other research
on Reflection.

I plan to somehow allocate more energy to this the next weeks (and maybe scale down on
Pharo bugfix harvesting. Note to self: Definitly I should not start any new things in addition).

One thing that is clear: In general, having X things doing the same is not going to work in the long run.

We need to concentrate to have one Ok instance of everything. This is true for GUI toolskits, Parsers,
ASTs, Compilers, Browsers... Than we can iterate on this one thing to make it better. Wasting engery to
maintain multiple instances of everything makes no sense.

        Marcus

--
Marcus Denker  -- http://www.marcusdenker.de
INRIA Lille -- Nord Europe. Team RMoD.


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Lukas Renggli
> But I *definitly* need the infrastructure... in some form. I need to get Reflectivity to
> work again, it's needed for at least Jorge's work and JB's work... and for other research
> on Reflection.

Helvetia needs it too. Now I am using the old compiler as a backend,
that works reasonably well. I have other problems getting it to work
in the latest Pharo now.

Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Marcus Denker-4

On Mar 4, 2010, at 10:58 AM, Lukas Renggli wrote:

>> But I *definitly* need the infrastructure... in some form. I need to get Reflectivity to
>> work again, it's needed for at least Jorge's work and JB's work... and for other research
>> on Reflection.
>
> Helvetia needs it too. Now I am using the old compiler as a backend,
> that works reasonably well. I have other problems getting it to work
> in the latest Pharo now.
>

indeed. In turn, Helvetia would actually simplify the implementation. E.g. Quasi-Quoting
is very nice when building AST snippits to be inserted.

And of course Bytesurgeon screams for a Language-Boxes DSL (even though not directly
related to Reflectivity).

With Johan, I plan to use Helvetia together with Reflectivity for some research project.

So... yes.

        Marcus

--
Marcus Denker  -- http://www.marcusdenker.de
INRIA Lille -- Nord Europe. Team RMoD.


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Lukas Renggli
> indeed. In turn, Helvetia would actually simplify the implementation. E.g. Quasi-Quoting
> is very nice when building AST snippits to be inserted.

The situation also has its advantages :-)

I recently rewrote the Quasiquoting bootstrap code in a new and much
simpler way. Instead of directly plugging it into the NewCompiler and
generating byte code, I reimplemented the quoting mechanism as a pure
parse tree transformation on the AST. Like this it is the whole thing
is independent of the actual compiler being used. The quote, unquote
and splice nodes can be transformed to traditional nodes as a
preprocessing step to any backend.

> And of course Bytesurgeon screams for a Language-Boxes DSL (even though not directly
> related to Reflectivity).

Yeah, but first I have to get everything working again. My demo image
is still a pre-closure one.

> With Johan, I plan to use Helvetia together with Reflectivity for some research project.

Cool. Keep me in the loop on that.

Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Marcus Denker-4

On Mar 4, 2010, at 11:30 AM, Lukas Renggli wrote:

>> indeed. In turn, Helvetia would actually simplify the implementation. E.g. Quasi-Quoting
>> is very nice when building AST snippits to be inserted.
>
> The situation also has its advantages :-)
>
> I recently rewrote the Quasiquoting bootstrap code in a new and much
> simpler way. Instead of directly plugging it into the NewCompiler and
> generating byte code, I reimplemented the quoting mechanism as a pure
> parse tree transformation on the AST. Like this it is the whole thing
> is independent of the actual compiler being used. The quote, unquote
> and splice nodes can be transformed to traditional nodes as a
> preprocessing step to any backend.
>

Very good! This means it fits nicely into the pure-AST model fo Reflectivity...
(where bytecode is just an execution artefact, like the machine code in a JIT, but never
used as the model of Reflection or presented to the programer in any form)

>
>> With Johan, I plan to use Helvetia together with Reflectivity for some research project.
>
> Cool. Keep me in the loop on that.
>
Will do.

        Marcus

--
Marcus Denker  -- http://www.marcusdenker.de
INRIA Lille -- Nord Europe. Team RMoD.


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Alexandre Bergel
In reply to this post by Marcus Denker-4
> But I *definitly* need the infrastructure... in some form. I need to  
> get Reflectivity to
> work again, it's needed for at least Jorge's work and JB's work...  
> and for other research
> on Reflection.


I also needs it for my work on code profiling. I am currently using  
the "object as compiled method" trick to instrument the base  
application. It slows everything down significantly.

Alexandre

--
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.






_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Stéphane Ducasse
In reply to this post by Jorge Ressia
jorge

could you make a list of things to do in 10 min?
So that we get an idea. I really believe that we should invest for the future.

Stef

On Mar 4, 2010, at 10:07 AM, Jorge Ressia wrote:

> Yes, the New compiler is not better. However, I think the New Compiler
> is the right direction to pursue.
>
> After fighting with it for some months to make the Closures work in it
> I can tell you that I learn a great deal from it, painfully :) .
> But I now have a pretty good idea of what it is required to have a
> modularized compiler with more suitable abstraction for this
> particular domain.
> I implemented many tests and I have to implement a lot more. Then, I
> believe that by expressing ourselves through these tests we will be
> able to modify the compiler to make it look the way we want.
>
> At least that is my plan with the New compiler.
>
> Cheers,
>
> Jorge
>
> On Thu, Mar 4, 2010 at 9:53 AM, Lukas Renggli <[hidden email]> wrote:
>>>> I'll try to build the tests and come up with a better solution.
>>>
>>> You what I would love to spend 2/3 weeks just coding in the new compiler and making sure
>>> that it has the right abstractions/messages to deal with issue like that.
>>
>> The problem is that the New Compiler is not really in a better state
>> than the old compiler. Of course it has more objects and is much
>> easier to extend, but all in all it is a mess too. This is mostly due
>> to the fact that the system below it changed quite a bit in the past
>> years (pragmas, properties, closures, primitives, new bytecodes, ...)
>> and that the New Compiler had to be patched and changed quite heavily
>> to accommodate these new requirements that it was not designed for.
>> Maintaining and fixing the New Compiler got so extremely expensive
>> that it is questionable if this is still a viable platform? Ask Jorge
>> and Marcus.
>>
>> Lukas
>>
>> --
>> Lukas Renggli
>> http://www.lukas-renggli.ch
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Jorge Ressia
Hi,

Sure

- We need a way of testing the New compiler, It is very hard to now if
the changes I am introducing work. I have a set of test but they are
mainly oriented towards the closure implementation.
- Not everything is done with visitors.
- The ASTTranslator and subclasses have methods of considerable size
which are hard to understand, mainly the emits and accepts
- The Decompiler does not work
- IRTranslator: needs some work, there are some abstractions that are
not clear like sequences and pending instructions which make the code
in every other method more complicated.
- It is not completely modular, we do not have a way of configuring it
from scratch
- The phases of the compiler are not clearly defined

If you ask me, I would say that the key point is to have a way of
validating our compiler. This is what is holding me back from
introducing more aggressive refactorings.

This does not mean that we need all those point in order to have a
running compiler, on the contrary, the compiler is working quite well,
I am fixing special cases right now. But, if we what something we
could improve and experiment with, we will need these points.


Cheers,

Jorge

On Thu, Mar 4, 2010 at 1:16 PM, Stéphane Ducasse
<[hidden email]> wrote:

> jorge
>
> could you make a list of things to do in 10 min?
> So that we get an idea. I really believe that we should invest for the future.
>
> Stef
>
> On Mar 4, 2010, at 10:07 AM, Jorge Ressia wrote:
>
>> Yes, the New compiler is not better. However, I think the New Compiler
>> is the right direction to pursue.
>>
>> After fighting with it for some months to make the Closures work in it
>> I can tell you that I learn a great deal from it, painfully :) .
>> But I now have a pretty good idea of what it is required to have a
>> modularized compiler with more suitable abstraction for this
>> particular domain.
>> I implemented many tests and I have to implement a lot more. Then, I
>> believe that by expressing ourselves through these tests we will be
>> able to modify the compiler to make it look the way we want.
>>
>> At least that is my plan with the New compiler.
>>
>> Cheers,
>>
>> Jorge
>>
>> On Thu, Mar 4, 2010 at 9:53 AM, Lukas Renggli <[hidden email]> wrote:
>>>>> I'll try to build the tests and come up with a better solution.
>>>>
>>>> You what I would love to spend 2/3 weeks just coding in the new compiler and making sure
>>>> that it has the right abstractions/messages to deal with issue like that.
>>>
>>> The problem is that the New Compiler is not really in a better state
>>> than the old compiler. Of course it has more objects and is much
>>> easier to extend, but all in all it is a mess too. This is mostly due
>>> to the fact that the system below it changed quite a bit in the past
>>> years (pragmas, properties, closures, primitives, new bytecodes, ...)
>>> and that the New Compiler had to be patched and changed quite heavily
>>> to accommodate these new requirements that it was not designed for.
>>> Maintaining and fixing the New Compiler got so extremely expensive
>>> that it is questionable if this is still a viable platform? Ask Jorge
>>> and Marcus.
>>>
>>> Lukas
>>>
>>> --
>>> Lukas Renggli
>>> http://www.lukas-renggli.ch
>>>
>>> _______________________________________________
>>> Pharo-project mailing list
>>> [hidden email]
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Adrian Lienhard
In reply to this post by Jorge Ressia
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 <[hidden email]> 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
>>> [hidden email]
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

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

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

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


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

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



--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

Lukas Renggli
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
>>>>>>> [hidden email]
>>>>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Pharo-project mailing list
>>>>>> [hidden email]
>>>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Pharo-project mailing list
>>>>> [hidden email]
>>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>>
>>>>
>>>> _______________________________________________
>>>> Pharo-project mailing list
>>>> [hidden email]
>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>>
>>>
>>> _______________________________________________
>>> Pharo-project mailing list
>>> [hidden email]
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> 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
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: [update 1.1] #11243

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