Mantis - Squeak
Viewing Issue Advanced Details
7093 Compiler minor always 06-13-08 01:24 02-06-11 23:48
nicolas cellier  
nicolas cellier  
normal  
closed 3.10  
fixed  
none    
none trunk  
0007093: to:do: optimization is wrong when the block modifies the limit
This loop would not end without a halt if compiled by regular (old) Compiler:

| n |
n := 2.
1 to: n do: [:i | (n := n+1)>10 ifTrue: [self halt: 'Should I get here?']].
^n

When you think in term of message send, well no, you should not get here.
Of course, with parentheses you won't get there:

| n |
n := 2.
(1 to: n) do: [:i | (n := n+1)>10 ifTrue: [self halt: 'You won''t get there']].
^n

Nor:

| n |
1 to: (n := 2) do: [:i | (n := n+1)>10 ifTrue: [self halt: 'Nor will you get there']].
^n

Amazing isn't it?
Just as tricky, check this:

| n |
n := 2.
1 to: n by: 1 do: [:i | (n := n+1)>10 ifTrue: [self halt: 'Should I get here?']].
^n

But, subtle difference:

| n inc |
n := 2.
inc := 1.
1 to: n by: inc do: [:i | (n := n+1)>10 ifTrue: [self halt: 'You won''t get there']].
^n

Problem is located into MessageNode>>transformToDo:
Of course, if one corrects this bug, there is a pending Decompiler issue:

testDecompileToDoWithMovingLimit
    | n i |
    n := 4.
    i := 1.
    [i <= n] whileTrue: [
        n := n - 1.
        i := i + 1].

would decompile into:
    | n |
    n := 4.
    1
        to: n
        do: [:i | n := n - 1]
related to 0006456closed andreas [BUG] Interval of Float do: infinite loop 
 M7093-MessageNode-transformToDo.st [^] (2,596 bytes) 09-07-10 22:14

Notes
(0012296)
nicolas cellier   
06-13-08 01:54   
If optimization of to:do: is to change, think of 0006456
(0013864)
nicolas cellier   
09-07-10 19:21   
The problem still exists in trunk...
Following code halts when it should not.

| n |
n := 2.
1 to: n do: [:i | (n := n+1)>10 ifTrue: [self halt: 'Should I get here?']].
^n
(0013870)
nicolas cellier   
09-07-10 22:15   
Eliot fixed the Compiler bug, file attached.
(0013871)
nicolas cellier   
09-07-10 23:14   
http://source.squeak.org/trunk/Tests-nice.93.mcz [^]
Provides two non regression tests
- 1 for compiler
- 1 for decompiler
(0013872)
nicolas cellier   
09-07-10 23:38   
http://source.squeak.org/trunk/Compiler-nice.173.mcz [^] includes Compiler fix

The idea is to avoid optimizing a to:do: loop that does modify its limit inside the loop.

Note: an alternate solution would be to create a shadow variable, assign it with the limit, and optimize the block.

Note 2: the bug report also has a so far uncorrected Decompiler part and cannot be closed yet.
(0014044)
nicolas cellier   
02-03-11 23:32   
Fixed during 4.2 in http://source.squeak.org/trunk/Compiler-nice.184.mcz [^]