Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

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

Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Status: Accepted
Owner: [hidden email]

New issue 4915 by [hidden email]: compiler should answer the same  
result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

A friend of mine ask me the following


|result|
result := String new.
1 to: 10 do: [:n | result := result, n printString, ' '].

I am trying to do or print this code. It prints 'nil' instead of a  
collection. Its an example from PBE book.
do you know the reason?

although this one's working:

result := String new.
(1 to: 10) do: [:n | result := result, n printString, ' '].

I checked the implementation

Interval>>do: aBlock
        "Evaluate aBlock for each value of the interval.
        Implementation note: instead of repeatedly incrementing the value
                aValue := aValue + step.
        until stop is reached,
        We prefer to recompute value from start
                aValue := start + (index * step).
        This is better for floating points accuracy, while not degrading Integer  
and Fraction speed too much.
        Moreover, this is consistent with methods #at: and #size"

        | aValue index size |
        index := 0.
        size := self size.
        [index < size]
                whileTrue: [aValue := start + (index * step).
                        index := index + 1.
                        aBlock value: aValue]


Number>>to: stop do: aBlock
        "Normally compiled in-line, and therefore not overridable.
        Evaluate aBlock for each element of the interval (self to: stop by: 1)."
        | nextValue |
        nextValue := self.
        [nextValue <= stop]
                whileTrue:
                        [aBlock value: nextValue.
                        nextValue := nextValue + 1]





Well, the inlined to:do: doesn't behave the same as non inlined one:

|result|
result := String new.
1 to: 10 do: [:n | result := result, n printString, ' '] yourself.

--> 1

The inlined byte code is explicitely returning nil (at byte 60)

(Compiler new
        compileNoPattern: '|result|
result := String new.
1 to: 10 do: [:n | result := result, n printString, '' '']'
        in: Object context: nil notifying: nil
        ifFail: nil) generateWithTempNames.


37 <40> pushLit: String
38 <CC> send: new
39 <68> popIntoTemp: 0
40 <76> pushConstant: 1
41 <69> popIntoTemp: 1
42 <11> pushTemp: 1
43 <24> pushConstant: 10
44 <B4> send: <=
45 <AC 0D> jumpFalse: 60
47 <10> pushTemp: 0
48 <11> pushTemp: 1
49 <D2> send: printString
50 <E1> send: ,
51 <23> pushConstant: ' '
52 <E1> send: ,
53 <68> popIntoTemp: 0
54 <11> pushTemp: 1
55 <76> pushConstant: 1
56 <B0> send: +
57 <69> popIntoTemp: 1
58 <A3 EE> jumpTo: 42
60 <73> pushConstant: nil
61 <7C> returnTop

  IMO the compiler should answer the same result as the non-inlined one when  
the return value is asked for.  Arguably the non-inline version needs to be  
commented to specify that it returns the value it does (in this case self),  
and that the compiler's optimiser reflects this.


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Status: FixReviewNeeded
        Labels: Milestone-1.4 Difficulty-Easy

Comment #1 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

Just for the fun, and because in Smalltalk we can.

Name: SLICE-Issue-4915-value-of-inlined-todo-nice.1
Dependencies: Compiler-nice.316, CompilerTests-nice.99

Let an inlined to:do: answer the same result as a non inlined to:do:
This happens to be the receiver, and thus the initial value of the loop  
index.
So we can trivially
- push the value of init statement on the stack,
- remove the final nil that was pushed forValue,
- and leave the rest of code generation unchanged

This is a test and fix for  
http://code.google.com/p/pharo/issues/detail?id=4915

It may be partial because I didn't check the results of every other inlined  
statement.
At least, whileTrue: repeat and other loops return nil because they are all  
implemented with an inlined version, so they are OK (but it's absolutely  
un-guessable by a non expert).


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Labels: Type-Feature

Comment #2 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

(No comment was entered for this change.)


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Labels: -Milestone-1.4 Milestone-1.5

Comment #3 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

We will fix that in 1.5


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
In reply to this post by pharo
Updates:
        Status: MonkeyIsChecking

Comment #5 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915#c5

The Monkey is currently checking this issue. Please don't change it!


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Status: HumanReviewNeeded

Comment #6 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

(No comment was entered for this change.)


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Status: FixReviewNeeded

Comment #7 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

(No comment was entered for this change.)


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Status: MonkeyIsChecking

Comment #8 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915#c8

The Monkey is currently checking this issue. Please don't change it!


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Status: WorkNeeded

Comment #9 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915#c9

Monkey went bananas:
--------------------
Error while loading SLICE-Issue-4915-value-of-inlined-todo-nice.1 from  
http://ss3.gemstone.com/ss/PharoInbox:
        Error: No package found for selector sourceCode in class MethodNode
  1: RPackageOrganizer(Object)>>error:
  2: [self
                        error: ('No package found for selector {1} in class {2}' format:  
{aSelector. aClassNameSymbol})] in  
RPackageOrganizer>>packageDefiningOrExtendingSelector:inClassNamed:
  3: Array(Collection)>>detect:ifNone:
  4: RPackageOrganizer>>packageDefiningOrExtendingSelector:inClassNamed:
  5: RPackageOrganizer>>systemMethodRemovedActionFrom:
  6: WeakMessageSend>>value:
  7: WeakMessageSend>>cull:
  8: WeakMessageSend>>cull:cull:
  9: [action cull: anAnnouncement cull: announcer] in  
WeakAnnouncementSubscription>>deliver:
10: BlockClosure>>on:do:
        ...
----------------------------------------------------------
Loaded Source: SLICE-Issue-4915-value-of-inlined-todo-nice.1 from  
http://ss3.gemstone.com/ss/PharoInbox
Tested using Pharo-2.0-20233-a on CoInterpreter  
VMMaker-oscog-EstebanLorenzano.161 uuid:  
8e0c22c3-b48d-4d8d-a7f9-8a75dc246f28 Jul 19 2012
StackToRegisterMappingCogit VMMaker-oscog-EstebanLorenzano.161 uuid:  
8e0c22c3-b48d-4d8d-a7f9-8a75dc246f28 Jul 19 2012
https://git.gitorious.org/cogvm/blessed.git Commit:  
5151310c41b08f55b70e9c6250711cb1f3672ce7 Date: 2012-07-18 14:06:15 +0200  
By: Camillo Bruni <[hidden email]>


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo

Comment #10 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

I said just for fun, but the fact that the Rpackage keep feeding monkeys  
with bananas does not amuse me so much. At the end, integrating this change  
in Pharo will take 10x + time and effort than integrating in Squeak  
trunk... I hope the ROI will be there someday after all these refactorings,  
but it's obviously too soon.


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Status: FixReviewNeeded

Comment #11 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

squeak never changes... so yes, comparing to not integration, 1 single  
integration takes lim->inf amount more effort...


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Status: MonkeyIsChecking

Comment #12 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915#c12

The Monkey is currently checking this issue. Please don't change it!


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
In reply to this post by pharo

Comment #14 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

I could not load the slice because Compiler-MarcusDenker.315 is not found.


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo

Comment #15 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

OK I added Pharo14


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Status: FixToInclude

Comment #16 on issue 4915 by [hidden email]: compiler should  
answer the same result as the non-inlined one when the return value is  
asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

For me the issue is to include
- Test written, green.
- I've recompiled the whole image + run a bunch of tests and went ok

I provide a new slice for Pharo2.0


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

Re: Issue 4915 in pharo: compiler should answer the same result as the non-inlined one when the return value is asked for.

pharo
Updates:
        Status: Integrated

Comment #17 on issue 4915 by [hidden email]: compiler should answer  
the same result as the non-inlined one when the return value is asked for.
http://code.google.com/p/pharo/issues/detail?id=4915

Integrated in 20344

Diff information:
http://ss3.gemstone.com/ss/Pharo20/CompilerTests-StephaneDucasse.124.diff
http://ss3.gemstone.com/ss/Pharo20/Compiler-StephaneDucasse.350.diff



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker