[Pharo-project] [update 1.1] #11243
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
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
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
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 <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
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
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 <renggli@gmail.com> 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 Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
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 <renggli@gmail.com> 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 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
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 <stephane.ducasse@inria.fr> 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 <renggli@gmail.com> 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 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
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.
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
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.
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
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.
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 ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.
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
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
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
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
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
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
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
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
Thanks. Apparently the mailing-list is working again. Stef On Mar 8, 2010, at 3:13 PM, Lukas Renggli 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
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
participants (6)
-
Adrian Lienhard -
Alexandre Bergel -
Jorge Ressia -
Lukas Renggli -
Marcus Denker -
Stéphane Ducasse