Re: Issue 709 in pharo: Debugger highlight bug

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

Re: Issue 709 in pharo: Debugger highlight bug

pharo

Comment #19 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

Still behaves strangely in #14099. Slightly different behaviour.  
Highlighting breaks after a few steps. Method can't be restarted fails with  
DebuggerMethodMapForClosureCompiledMethods>>privateDeference:in:.

I'll dig around but any experts let me know! --Mike (@ESUG)


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo
Updates:
        Labels: Milestone-1.3 Milestone-1.4

Comment #20 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

transformToDo: encoder
        " var := rcvr. L1: [var <= arg1] Bfp(L2) [block body. var := var + inc]
Jmp(L1) L2: "
        | limit increment block initStmt test incStmt limitInit blockVar myRange  
blockRange limitIsAssignedTo |
        "First check for valid arguments"
        ((arguments last isMemberOf: BlockNode)
          and: [arguments last numberOfArguments = 1
          and: [arguments last firstArgument isVariableReference "As with debugger  
remote vars"]]) ifFalse:
                [^false].
        arguments size = 3
                ifTrue: [increment := arguments at: 2.
                                (increment isConstantNumber
                                 and: [increment literalValue ~= 0]) ifFalse: [^false]]
                ifFalse: [increment := encoder encodeLiteral: 1].
        (limit := arguments at: 1) isVariableReference ifTrue:
                [limitIsAssignedTo := false.
                 arguments last nodesDo:
                        [:node|
                        (node isAssignmentNode and: [node variable = limit]) ifTrue:
                                [limitIsAssignedTo := true]].
                 limitIsAssignedTo ifTrue:
                        [^false]].
        arguments size < 3 ifTrue:   "transform to full form"
                [selector := SelectorNode new key: #to:by:do: code: #macro].

        "Now generate auxiliary structures"
        myRange := encoder rawSourceRanges at: self ifAbsent: [1 to: 0].
        block := arguments last.
        blockRange := encoder rawSourceRanges at: block ifAbsent: [1 to: 0].
        blockVar := block firstArgument.
        initStmt := AssignmentNode new variable: blockVar value: receiver.
        limit isVariableReference | limit isConstantNumber
                ifTrue: [limitInit := nil]
                ifFalse:  "Need to store limit in a var"
                        [limit := encoder bindBlockArg: blockVar key, 'LimiT' within: block.
                         limit scope: -2.  "Already done parsing block; flag so it won't print"
                         block addArgument: limit.
                         limitInit := AssignmentNode new
                                                        variable: limit
                                                        value: arguments first].
        test := MessageNode new
                               receiver: blockVar
                               selector: (increment key > 0 ifTrue:
[#<=] ifFalse: [#>=])
                               arguments: (Array with: limit)
                               precedence: precedence from: encoder
                               sourceRange: (myRange first to: blockRange  
last).
        incStmt := AssignmentNode new
                                variable: blockVar
                                value: (MessageNode new
                                                        receiver: blockVar selector: #+
                                                        arguments: (Array with: increment)
                                                        precedence: precedence from: encoder)
                                from: encoder
                                sourceRange: (myRange last to: myRange last).
        arguments := (Array with: limit with: increment with: block),
                                        (Array with: initStmt with: test with: incStmt with: limitInit).
        block noteOptimizedIn: self.
        ^true


nicolas proposed this fix and it seems to work on my image.


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #21 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

Helper class: DebuggerAnalyser

example
DebuggerAnalyser inspectOn: #toDoOutsideTemp.

in the inspector
self stepOver

Attachments:
        DebuggerAnalyser.st  12.0 KB


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #22 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

Helper class: DebuggerAnalyser

example
DebuggerAnalyser inspectOn: #toDoOutsideTemp.

in the inspector
self stepOver

Attachments:
        DebuggerAnalyser.st  12.0 KB


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #23 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

"Fix" isn't quite complete. Use the analyser to step through (self  
stepOver) and you can see a few issues.

1. first assignment before first unary send.
2. Loop is not highlighted first, the assignment inside the loop is  
immediately highlighted

but the end loop point looks more correct


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo
Updates:
        Labels: -Milestone-1.3

Comment #24 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

I think this was broken for a loooong time, so I take the liberty of  
removing the 1.3 tag.

(If someone later submits a ready made fix for 1.3, there is no problem in  
including it. But it's not a show stopper)


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #25 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

np Marcus.
Elliot points out that the "first assignment" bug is not present in Squeak.  
so this is something we can focus on to attempt to find the difference in  
the code. I have confirmed this.
--Mike



_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #26 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

Update send to the list:

So I found the bug with the first assignment. It is caused by a difference  
between squeak and Pharo in SortedCollection. (on my phone so will be brief)

In Pharo the debugger map collection gets sorted and asked for an index for  
a given pc. It is supposed to answer 1 because there is no match and it is  
expected to be the first index for insertion. However when the  
asSortedCollection gets done the internal start index of the collection is  
2. This manifests the bug. 2 is answered as the insertion position but this  
is a value internal to the collection.

Squeak does not have this bug because it forces a reset of the start index  
to 1. Hope that makes sense. I don't know what the correct fix is. We copy  
the reset:1 type code to force the collection to have start from 1  
property. We check semantics of index for insertion. Is it the position of  
the element of index relative to pointers? Or we change the debugger to  
calculate the offset (but this feels wrong)

So this is an opportunity for some test cases and fixes around sorted  
collection if we know what the correct semantics are. This leak of '2' out  
of the collection is only wrong if 2 is a valid state. If that makes  
sense...

Cheers
Mike


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #27 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

Hum, indexForInserting: is _supposed_ to answer the internal index...
Depending on the startIndex being 1 for doing the correct at: later is just  
wrong.
Squeak potentially has the same highlight bug for different methds, as  
insert:before: used when converting to SortedCollection in no way ensures  
startIndex remains 1 just because it started as it.

I assume it was written this way for performance reasons, now that we have  
#findBinary: aBlock, we can rewrite the code achieving same performance  
using an ordinary collection, something like:

"Maintain a sorted array of intervals"
sortedSourceMap ifNil: [sortedSourceMap := self abstractSourceMap values  
sorted].

"Look up which interval contains pc using a binary search"
sortedSourceMap findBinary: [: one  |
         (one includes: pc)
             ifTrue: [0]
             ifFalse: [one first > pc ifTrue: [-1] ifFalse: [1]] ]

(with some extra code needed to deal with the cases where pc < first  
interval or > last interval)


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #28 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

For the "first assignment" bug:

Here's a changeset with the 2 new methods, plus the resulting  
DebuggerMethodMap>>#rangeForPC:contextIsActiveContext:

See http://forum.world.st/Debugger-bug-fix-1-tp3798667p3799646.html for  
details.

For a less radical restructuring of the code, Eliots fix from:
http://forum.world.st/Debugger-bug-fix-1-tp3798667p3799497.html

is also a good solution.

Attachments:
        DebuggerMethodMap.HenrikSperreJohansen.1.cs  1.3 KB


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #29 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

And since I wrote the code in a mail rather than in an image, of course  
there was a bug...
indexForInserting: could never be < 1, so the check was unnecessary.
In case PC is < that found in debuggermap, first range is always selected.
(Not sure if this is entirely appropriate, as you have to step an extra  
time to actually pass the expression. In that regards, not doing  
highlighting at all when entering the method was more appropriate).

If we explicitly check for this, like in the attached .cs, any ifNone:  
invocations are due to being off the end, so we return the same range as  
previously for that.

Apart from the buggy index lookup, the attached .cs should fully reflect  
the original intention of the code.

Attachments:
        DebuggerMethodMap.2.cs  1.5 KB


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #30 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

And since I wrote the code in a mail rather than in an image, of course  
there was a bug...
indexForInserting: could never be < 1, so the check was unnecessary.
In case PC is < that found in debuggermap, first range is always selected.
(Not sure if this is entirely appropriate, as you have to step an extra  
time to actually pass the expression. In that regards, not doing  
highlighting at all when entering the method was more appropriate).

If we explicitly check for this, like in the attached .cs, any ifNone:  
invocations are due to being off the end, so we return the same range as  
previously for that.

Apart from the buggy index lookup, the attached .cs should fully reflect  
the original intention of the code.

Attachments:
        DebuggerMethodMap.2.cs  1.5 KB


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo
Updates:
        Status: FixProposed

Comment #31 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

(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 709 in pharo: Debugger highlight bug

pharo
Updates:
        Status: Comment

Comment #32 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

in 14130

Can this issue be closed?


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo
Updates:
        Labels: Milestone-1.3

Comment #33 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

And: do we add it to 1.3, too?


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #34 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

(What I added to 1.4 is DebuggerMethodMap.2.cs )


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo

Comment #35 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

No, there are several other highlighting issues covered by this issue not  
fixed yet.


_______________________________________________
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 709 in pharo: Debugger highlight bug

pharo
Updates:
        Labels: -Milestone-1.3

Comment #36 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

(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 709 in pharo: Debugger highlight bug

pharo
Updates:
        Labels: -Type-Closure Type-Bug

Comment #37 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

(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 709 in pharo: Debugger highlight bug

pharo
Updates:
        Status: Workneeded

Comment #38 on issue 709 by [hidden email]: Debugger highlight bug
http://code.google.com/p/pharo/issues/detail?id=709

(No comment was entered for this change.)


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