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 |
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 |
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 |
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 |
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 |
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 |
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 |
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 |
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 |
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 |
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 |
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 |
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 |
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 |
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 |
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 |
Free forum by Nabble | Edit this page |