Mantis - Squeak
Viewing Issue Advanced Details
6720 Tools major always 10-10-07 22:36 12-17-10 22:17
nicolas cellier  
 
normal  
closed 3.10  
fixed  
none    
none trunk  
0006720: Using twice the same block argument names and auto accepting temps can put the Debugger in a mess
Debugger can get completely lost about temporary variable slots when we use several time the same names for block args
       AND
when we auto accept to declare a temporary.

HOW TO REPRODUCE?
Open a browser, say on Object, and accept following method (see below).
Declare warningMessage as a temporary when you are asked to do so.
Then execute (nil testWeirdDebuggerCase), open a debugger when assertion fails and inspect the temps... OUCH

If you doIt in a workspace, or accept with well declared temps, the bug does not show. What the tricky kind it must be!
 
testWeirdDebuggerCase
    | compareSelectors theNaN anotherNaN comparand brokenMethods |
    compareSelectors := #(#< #<= #> #>= #=).
    theNaN := Float nan.
    anotherNaN := Float infinity - Float infinity.
    comparand := {1. 2.3. Float infinity. 2/3. 1.25s2. 2 raisedTo: 50}.
    comparand := comparand , (comparand collect: [:e | e negated]).
    comparand := comparand , {theNaN. anotherNaN}.

"do a first pass to collect all broken methods"
    brokenMethods := Set new.
    comparand do: [:comp |
        compareSelectors do: [:op |
            (theNaN perform: op with: comp) ifTrue: [brokenMethods add: (theNaN class lookupSelector: op)].
            (comp perform: op with: theNaN) ifTrue: [brokenMethods add: (comp class lookupSelector: op)]].
        (theNaN ~= comp) ifFalse: [brokenMethods add: (theNaN class lookupSelector: #~=)].
        (comp ~= theNaN) ifFalse: [brokenMethods add: (comp class lookupSelector: #~=)]].
    
"build a warning message to tell about all broken methods at once"
    warningMessage := String streamContents: [:s |
            s nextPutAll: 'According to IEEE 754 comparing with a NaN should always return false, except ~= that should return true.'; cr.
            s nextPutAll: 'All these methods failed to do so. They are either broken or call a broken one'.
            brokenMethods do: [:e | s cr; print: e methodClass; nextPutAll: '>>'; print: e selector]].
        
"Redo the tests so as to eventually open a debugger on one of the failures"
    brokenMethods := Set new.
    comparand do: [:comp |
        compareSelectors do: [:op |
            self assert: (theNaN perform: op with: comp) = false description: warningMessage.
            self assert: (comp perform: op with: theNaN) = false description: warningMessage].
        self assert: (theNaN ~= comp) description: warningMessage.
        self assert: (comp ~= theNaN) description: warningMessage].

Notes
(0011296)
nicolas cellier   
10-10-07 23:14   
Seems like the guilty is Encoder>>#bindAndJuggle:

This one states that:
"Declared temps must precede block temps for decompiler and debugger to work right"

Block temps should have scope > 0, and this method search first such slot.

I suspect that Declared temps SHOULD ALSO precede block args for decompiler and debugger to work right. But block args currently have a scope of -1, so they will precede auto-declared temps.

Can an expert in the old Compiler arcanes confirm what i say?
(0013948)
leves   
11-25-10 19:44   
I guess this was fixed by the closure changes.
(0013992)
nicolas cellier   
12-17-10 22:17   
Cannot reproduce the error in trunk 4.2.
Fixed by various closure/compiler changes, probably in 4.1.