Some bugs

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

Re: Some bugs

Nicolas Cellier
So the entries of interest for highlighting are

Debugger>>contentsSelection
Debugger>>pcRange
CompiledMethod>>debuggerMap
DebuggerMethodMap class>>forMethod:
DebuggerMethodMap>>rangeForPC:contextIsActiveContext:

Then you see the DebuggerMethodMap>>forMethod:methodNode: takes both a
CompiledMethod and its #methodNode.
CompiledMethod>>methodNode invokes the Parser to get the
AbstractSyntaxTree from method source, and if it ever fails ends up by
trying to decompile the byteCodes.

This is the easy part. Now we to deal with #abstractPCForConcretePC:
and #abstractSourceMap.

By reading CompiledMethod>>abstractPCForConcretePC: you should quickly
understand that a concrete PC is a byte offset of current byteCode
(the offsets displayed in the byteCode view) while the abstractPC is
just the rank of current byteCode in the list of byteCodes
instructions composing the CompiledMethod. This is just because
"byteCodes" may spread on several bytes beside their name...
This will use InstructionStream and InstructionClient which are just
an iterator and a sort of visitor on byteCode instructions.
So this is not really interesting.

The more interesting part is #abstractSourceMap
There is a first step to obtain CompiledMethod>>rawSourceRangesAndMethodDo:
This is the most important part.
The rest is again a mapping from concretePC (instruction byte offset)
to abstractPC (instruction rank).
And some build of a dictionary mapping instruction rank (abstractPC)
-> selected range.

Note that the last trick seems to use a regenerated CompiledMethod
(theMethodToScan) rather than the original CompiledMethod. There is no
assertion whether these two are equivalent or not. A priori, they
should, unless the Compiler changed since last compilation or if its
behaviour is affected by some Preferences... Would we introduce some
customizable Compiler optimizations that this could become a problem
(We would then add to map decompiled AST to source code AST, probably
with guesses, unless the CompiledMethod would contain debugger
friendly hints...)
We will consider this is not a problem by now.

So let's now concentrate on rawSourceRangesAndMethodDo:
The nice thing is that you now can just debug this

(ClosureTests>>#testToDoOutsideTemp) methodNode
rawSourceRangesAndMethodDo: [:rs :mth | ]

and see how it goes in Squeak. I did not look in Pharo yet, but I
would be amazed to see it much different.
It's now late, and my spare time is off, but you have clues to get
more insights. I wish you good debugging, and come back to me if it
ever goes in deeper complications.

Cheers

Nicolas

2011/8/24 Michael Roberts <[hidden email]>:

>
>>
>> Ok I'm curious to know then.
>
> Here is a little trace from this example method:
>
> toDoOutsideTemp
> | temp collection |
> collection := OrderedCollection new.
> 1 to: 5 do: [ :index |
> temp := index.
> collection add: [ temp ] ]
>
> Trace is start,stop position of the highlight for each 'step over'.
>
> Whilst the numbers are hard to visualise, below you can see how they
> slightly diverge.
> Left Pharo  Right  Squeak
>
> 50, 73     71, 73       diff
> 71, 73     71, 73
> 50, 73     50, 73
> 108, 115   79, 121      diff
> 79, 121    79, 121
> 108, 115   108, 115
> 132, 144   132, 144
> 147, 146   146, 146     (diff negative size means no highlight)
> 146, 146   146, 146
> 79, 121    79, 121
> 108, 115   108, 115
> 132, 144   132, 144
> 147, 146   146, 146
> 146, 146   146, 146
> 79, 121    79, 121
> 108, 115   108, 115
> 132, 144   132, 144
> 147, 146   146, 146
> 146, 146   146, 146
> 79, 121    79, 121
> 108, 115   108, 115
> etc...
> For example the first difference is because Pharo shows the whole assignment
> of the first line as the first send, even though it is not.
> The second difference is that Pharo shows the assignment inside the block as
> the first highlight of the loop even though the to:do should be
> highlighted....but both Pharo & Squeak get the to:do: wrong when they choose
> to show it.
> hope you get the idea...
> Mike

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Nicolas Cellier
2011/8/24 Nicolas Cellier <[hidden email]>:

> So the entries of interest for highlighting are
>
> Debugger>>contentsSelection
> Debugger>>pcRange
> CompiledMethod>>debuggerMap
> DebuggerMethodMap class>>forMethod:
> DebuggerMethodMap>>rangeForPC:contextIsActiveContext:
>
> Then you see the DebuggerMethodMap>>forMethod:methodNode: takes both a
> CompiledMethod and its #methodNode.
> CompiledMethod>>methodNode invokes the Parser to get the
> AbstractSyntaxTree from method source, and if it ever fails ends up by
> trying to decompile the byteCodes.
>
> This is the easy part. Now we to deal with #abstractPCForConcretePC:
> and #abstractSourceMap.
>
> By reading CompiledMethod>>abstractPCForConcretePC: you should quickly
> understand that a concrete PC is a byte offset of current byteCode
> (the offsets displayed in the byteCode view) while the abstractPC is
> just the rank of current byteCode in the list of byteCodes
> instructions composing the CompiledMethod. This is just because
> "byteCodes" may spread on several bytes beside their name...
> This will use InstructionStream and InstructionClient which are just
> an iterator and a sort of visitor on byteCode instructions.
> So this is not really interesting.
>
> The more interesting part is #abstractSourceMap
> There is a first step to obtain CompiledMethod>>rawSourceRangesAndMethodDo:
> This is the most important part.
> The rest is again a mapping from concretePC (instruction byte offset)
> to abstractPC (instruction rank).
> And some build of a dictionary mapping instruction rank (abstractPC)
> -> selected range.
>
> Note that the last trick seems to use a regenerated CompiledMethod
> (theMethodToScan) rather than the original CompiledMethod. There is no
> assertion whether these two are equivalent or not. A priori, they
> should, unless the Compiler changed since last compilation or if its
> behaviour is affected by some Preferences... Would we introduce some
> customizable Compiler optimizations that this could become a problem
> (We would then add to map decompiled AST to source code AST, probably
> with guesses, unless the CompiledMethod would contain debugger
> friendly hints...)
> We will consider this is not a problem by now.
>
> So let's now concentrate on rawSourceRangesAndMethodDo:
> The nice thing is that you now can just debug this
>
> (ClosureTests>>#testToDoOutsideTemp) methodNode
> rawSourceRangesAndMethodDo: [:rs :mth | ]
>
> and see how it goes in Squeak. I did not look in Pharo yet, but I
> would be amazed to see it much different.
> It's now late, and my spare time is off, but you have clues to get
> more insights. I wish you good debugging, and come back to me if it
> ever goes in deeper complications.
>
> Cheers
>
> Nicolas
>

Oh, I couldn't resist and just tried, and it's indeed getting harder,
because it involves a zoo of Compiler internals, at least the Encoder,
the BlockAnalyzer...
Funny, the method node is once more generated in this phase :). All
this stuff is cached at upper level, but we have at least clues for
further optimizations ;)

Nicolas

> 2011/8/24 Michael Roberts <[hidden email]>:
>>
>>>
>>> Ok I'm curious to know then.
>>
>> Here is a little trace from this example method:
>>
>> toDoOutsideTemp
>> | temp collection |
>> collection := OrderedCollection new.
>> 1 to: 5 do: [ :index |
>> temp := index.
>> collection add: [ temp ] ]
>>
>> Trace is start,stop position of the highlight for each 'step over'.
>>
>> Whilst the numbers are hard to visualise, below you can see how they
>> slightly diverge.
>> Left Pharo  Right  Squeak
>>
>> 50, 73     71, 73       diff
>> 71, 73     71, 73
>> 50, 73     50, 73
>> 108, 115   79, 121      diff
>> 79, 121    79, 121
>> 108, 115   108, 115
>> 132, 144   132, 144
>> 147, 146   146, 146     (diff negative size means no highlight)
>> 146, 146   146, 146
>> 79, 121    79, 121
>> 108, 115   108, 115
>> 132, 144   132, 144
>> 147, 146   146, 146
>> 146, 146   146, 146
>> 79, 121    79, 121
>> 108, 115   108, 115
>> 132, 144   132, 144
>> 147, 146   146, 146
>> 146, 146   146, 146
>> 79, 121    79, 121
>> 108, 115   108, 115
>> etc...
>> For example the first difference is because Pharo shows the whole assignment
>> of the first line as the first send, even though it is not.
>> The second difference is that Pharo shows the assignment inside the block as
>> the first highlight of the loop even though the to:do should be
>> highlighted....but both Pharo & Squeak get the to:do: wrong when they choose
>> to show it.
>> hope you get the idea...
>> Mike
>

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Michael Roberts-2
;-) thanks! yes i looked at it a little with Marcus and got lost when the InstructionStream was invoked.  Your analysis is really useful. i have a 4 hr train journey soon!

Mike

On Wed, Aug 24, 2011 at 10:08 PM, Nicolas Cellier <[hidden email]> wrote:
2011/8/24 Nicolas Cellier <[hidden email]>:
> So the entries of interest for highlighting are
>
> Debugger>>contentsSelection
> Debugger>>pcRange
> CompiledMethod>>debuggerMap
> DebuggerMethodMap class>>forMethod:
> DebuggerMethodMap>>rangeForPC:contextIsActiveContext:
>
> Then you see the DebuggerMethodMap>>forMethod:methodNode: takes both a
> CompiledMethod and its #methodNode.
> CompiledMethod>>methodNode invokes the Parser to get the
> AbstractSyntaxTree from method source, and if it ever fails ends up by
> trying to decompile the byteCodes.
>
> This is the easy part. Now we to deal with #abstractPCForConcretePC:
> and #abstractSourceMap.
>
> By reading CompiledMethod>>abstractPCForConcretePC: you should quickly
> understand that a concrete PC is a byte offset of current byteCode
> (the offsets displayed in the byteCode view) while the abstractPC is
> just the rank of current byteCode in the list of byteCodes
> instructions composing the CompiledMethod. This is just because
> "byteCodes" may spread on several bytes beside their name...
> This will use InstructionStream and InstructionClient which are just
> an iterator and a sort of visitor on byteCode instructions.
> So this is not really interesting.
>
> The more interesting part is #abstractSourceMap
> There is a first step to obtain CompiledMethod>>rawSourceRangesAndMethodDo:
> This is the most important part.
> The rest is again a mapping from concretePC (instruction byte offset)
> to abstractPC (instruction rank).
> And some build of a dictionary mapping instruction rank (abstractPC)
> -> selected range.
>
> Note that the last trick seems to use a regenerated CompiledMethod
> (theMethodToScan) rather than the original CompiledMethod. There is no
> assertion whether these two are equivalent or not. A priori, they
> should, unless the Compiler changed since last compilation or if its
> behaviour is affected by some Preferences... Would we introduce some
> customizable Compiler optimizations that this could become a problem
> (We would then add to map decompiled AST to source code AST, probably
> with guesses, unless the CompiledMethod would contain debugger
> friendly hints...)
> We will consider this is not a problem by now.
>
> So let's now concentrate on rawSourceRangesAndMethodDo:
> The nice thing is that you now can just debug this
>
> (ClosureTests>>#testToDoOutsideTemp) methodNode
> rawSourceRangesAndMethodDo: [:rs :mth | ]
>
> and see how it goes in Squeak. I did not look in Pharo yet, but I
> would be amazed to see it much different.
> It's now late, and my spare time is off, but you have clues to get
> more insights. I wish you good debugging, and come back to me if it
> ever goes in deeper complications.
>
> Cheers
>
> Nicolas
>

Oh, I couldn't resist and just tried, and it's indeed getting harder,
because it involves a zoo of Compiler internals, at least the Encoder,
the BlockAnalyzer...
Funny, the method node is once more generated in this phase :). All
this stuff is cached at upper level, but we have at least clues for
further optimizations ;)

Nicolas

> 2011/8/24 Michael Roberts <[hidden email]>:
>>
>>>
>>> Ok I'm curious to know then.
>>
>> Here is a little trace from this example method:
>>
>> toDoOutsideTemp
>> | temp collection |
>> collection := OrderedCollection new.
>> 1 to: 5 do: [ :index |
>> temp := index.
>> collection add: [ temp ] ]
>>
>> Trace is start,stop position of the highlight for each 'step over'.
>>
>> Whilst the numbers are hard to visualise, below you can see how they
>> slightly diverge.
>> Left Pharo  Right  Squeak
>>
>> 50, 73     71, 73       diff
>> 71, 73     71, 73
>> 50, 73     50, 73
>> 108, 115   79, 121      diff
>> 79, 121    79, 121
>> 108, 115   108, 115
>> 132, 144   132, 144
>> 147, 146   146, 146     (diff negative size means no highlight)
>> 146, 146   146, 146
>> 79, 121    79, 121
>> 108, 115   108, 115
>> 132, 144   132, 144
>> 147, 146   146, 146
>> 146, 146   146, 146
>> 79, 121    79, 121
>> 108, 115   108, 115
>> 132, 144   132, 144
>> 147, 146   146, 146
>> 146, 146   146, 146
>> 79, 121    79, 121
>> 108, 115   108, 115
>> etc...
>> For example the first difference is because Pharo shows the whole assignment
>> of the first line as the first send, even though it is not.
>> The second difference is that Pharo shows the assignment inside the block as
>> the first highlight of the loop even though the to:do should be
>> highlighted....but both Pharo & Squeak get the to:do: wrong when they choose
>> to show it.
>> hope you get the idea...
>> Mike
>


Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Michael Roberts-2
Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Stéphane Ducasse
In reply to this post by Nicolas Cellier
tx nicolas this is a cool way to help :)

Stef

On Aug 24, 2011, at 9:54 PM, Nicolas Cellier wrote:

> So the entries of interest for highlighting are
>
> Debugger>>contentsSelection
> Debugger>>pcRange
> CompiledMethod>>debuggerMap
> DebuggerMethodMap class>>forMethod:
> DebuggerMethodMap>>rangeForPC:contextIsActiveContext:
>
> Then you see the DebuggerMethodMap>>forMethod:methodNode: takes both a
> CompiledMethod and its #methodNode.
> CompiledMethod>>methodNode invokes the Parser to get the
> AbstractSyntaxTree from method source, and if it ever fails ends up by
> trying to decompile the byteCodes.
>
> This is the easy part. Now we to deal with #abstractPCForConcretePC:
> and #abstractSourceMap.
>
> By reading CompiledMethod>>abstractPCForConcretePC: you should quickly
> understand that a concrete PC is a byte offset of current byteCode
> (the offsets displayed in the byteCode view) while the abstractPC is
> just the rank of current byteCode in the list of byteCodes
> instructions composing the CompiledMethod. This is just because
> "byteCodes" may spread on several bytes beside their name...
> This will use InstructionStream and InstructionClient which are just
> an iterator and a sort of visitor on byteCode instructions.
> So this is not really interesting.
>
> The more interesting part is #abstractSourceMap
> There is a first step to obtain CompiledMethod>>rawSourceRangesAndMethodDo:
> This is the most important part.
> The rest is again a mapping from concretePC (instruction byte offset)
> to abstractPC (instruction rank).
> And some build of a dictionary mapping instruction rank (abstractPC)
> -> selected range.
>
> Note that the last trick seems to use a regenerated CompiledMethod
> (theMethodToScan) rather than the original CompiledMethod. There is no
> assertion whether these two are equivalent or not. A priori, they
> should, unless the Compiler changed since last compilation or if its
> behaviour is affected by some Preferences... Would we introduce some
> customizable Compiler optimizations that this could become a problem
> (We would then add to map decompiled AST to source code AST, probably
> with guesses, unless the CompiledMethod would contain debugger
> friendly hints...)
> We will consider this is not a problem by now.
>
> So let's now concentrate on rawSourceRangesAndMethodDo:
> The nice thing is that you now can just debug this
>
> (ClosureTests>>#testToDoOutsideTemp) methodNode
> rawSourceRangesAndMethodDo: [:rs :mth | ]
>
> and see how it goes in Squeak. I did not look in Pharo yet, but I
> would be amazed to see it much different.
> It's now late, and my spare time is off, but you have clues to get
> more insights. I wish you good debugging, and come back to me if it
> ever goes in deeper complications.
>
> Cheers
>
> Nicolas
>
> 2011/8/24 Michael Roberts <[hidden email]>:
>>
>>>
>>> Ok I'm curious to know then.
>>
>> Here is a little trace from this example method:
>>
>> toDoOutsideTemp
>> | temp collection |
>> collection := OrderedCollection new.
>> 1 to: 5 do: [ :index |
>> temp := index.
>> collection add: [ temp ] ]
>>
>> Trace is start,stop position of the highlight for each 'step over'.
>>
>> Whilst the numbers are hard to visualise, below you can see how they
>> slightly diverge.
>> Left Pharo  Right  Squeak
>>
>> 50, 73     71, 73       diff
>> 71, 73     71, 73
>> 50, 73     50, 73
>> 108, 115   79, 121      diff
>> 79, 121    79, 121
>> 108, 115   108, 115
>> 132, 144   132, 144
>> 147, 146   146, 146     (diff negative size means no highlight)
>> 146, 146   146, 146
>> 79, 121    79, 121
>> 108, 115   108, 115
>> 132, 144   132, 144
>> 147, 146   146, 146
>> 146, 146   146, 146
>> 79, 121    79, 121
>> 108, 115   108, 115
>> 132, 144   132, 144
>> 147, 146   146, 146
>> 146, 146   146, 146
>> 79, 121    79, 121
>> 108, 115   108, 115
>> etc...
>> For example the first difference is because Pharo shows the whole assignment
>> of the first line as the first send, even though it is not.
>> The second difference is that Pharo shows the assignment inside the block as
>> the first highlight of the loop even though the to:do should be
>> highlighted....but both Pharo & Squeak get the to:do: wrong when they choose
>> to show it.
>> hope you get the idea...
>> Mike
>


Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Nicolas Cellier
A few more notes:

- Encoder>>rawSourceRanges answer a mapping AST Node -> sourceRange
 Gnerally, I would expect sourceRange to be an Interval, but this has
to be confirmed.
 Theses ranges are constructed at source code Parse time (see senders
of #noteSourceRange:forNode:)
- The program counters are assigned to AST nodes at byte code #generate time
- for inlined macros, like #to:do: in our case, this pc is after the
initialize and test statements.

in CompiledMethod>>#rawSourceRangesAndMethodDo:
I just evaluated this:

methNode encoder rawSourceRanges collect: [:ival |
        sourceText copyFrom: ival first to: ival last]

And what I see is that the original to:do: message (before macros
inlining) has the right range
{1
        to: 5
        do: [:index |
                temp := index.
                collection
                        add: [temp]]}->a Text for 'to: 5 do: [ :index |
                temp := index.
                collection add: [ temp ] ]'
This node is the original #to:do: and has pc=42

There is also
{a LeafNode}->a Text for '[ :index |
                temp := index.
                collection add: [ temp ] ]'
which has a nil pc so it won't be taken into account in the source map.

But one of the messages produced by the inlining has this curious range:
{index <= 5}->a Text for 'to: 5 do: [ :index |
                temp := index.
                c'
This node has pc = 40, so it will be selected before the correct one
above has a chance to be.
{index <= 5} is the test statement produced by inlining, and this
seems to be the incorrect highlighting we see.
Thus we know we have to concentrate on macro inlining.
This happens in MessageNode>>transformToDo:
We'll see this.

In the interim, I played with eliminating the nodes having unitilialized pc
Let us try to evaluate this snippet in debugger's inspector:
(methNode encoder rawSourceRanges keys select: [:e | e pc > 0])
collect: [:node |
        | ival |
        ival := methNode encoder rawSourceRanges at: node.
        node -> (sourceText copyFrom: ival first to: ival last)]

Oh god, a bug in the debugger:
DoItIn: homeContext
        ^ ((homeContext namedTempAt: 2) encoder rawSourceRanges keys
                select: [:e | e pc > 0])
                collect: [:node |
                        | ival |
                        ival := (node namedTempAt: 2) encoder rawSourceRanges at: node.
                        node
                                -> (sourceText copyFrom: ival first to: ival last)]
(node namedTempAt: 2) does not mean a thing...
A confusion occurred between the homeContext (DoItIn: method argument)
and node (the block argument)
Nevermind... forget about it.

Now let's just concentrate on MessageNode>>transformToDo:
We see this code:
  test := MessageNode new
                                receiver: blockVar
                                selector: (increment key > 0 ifTrue: [#<=] ifFalse: [#>=])
                                arguments: (Array with: limit)
                                precedence: precedence from: encoder
                                sourceRange: (myRange first to: blockRange first).

So the intention seems to select 'to: 5 do: '

But we see this:
BlockNode>>noteSourceRangeStart:end:encoder:
        "Note two source ranges for this node.  One is for the debugger
         and is of the last expression, the result of the block.  One is for
         source analysis and is for the entire block."
        encoder
                noteSourceRange: (start to: end)
                forNode: self closureCreationNode.
        startOfLastStatement
                ifNil:
                        [encoder
                                noteSourceRange: (start to: end)
                                forNode: self]
                ifNotNil:
                        [encoder
                                noteSourceRange: (startOfLastStatement to: end - 1)
                                forNode: self]

So it seems intentional to select only the last instruction of the
block for the debugger.
We'd better not change this without prior asking Eliot.
But obviously, this is not what is expected by the #transformToDo:

Nicolas

2011/8/25 Stéphane Ducasse <[hidden email]>:

> tx nicolas this is a cool way to help :)
>
> Stef
>
> On Aug 24, 2011, at 9:54 PM, Nicolas Cellier wrote:
>
>> So the entries of interest for highlighting are
>>
>> Debugger>>contentsSelection
>> Debugger>>pcRange
>> CompiledMethod>>debuggerMap
>> DebuggerMethodMap class>>forMethod:
>> DebuggerMethodMap>>rangeForPC:contextIsActiveContext:
>>
>> Then you see the DebuggerMethodMap>>forMethod:methodNode: takes both a
>> CompiledMethod and its #methodNode.
>> CompiledMethod>>methodNode invokes the Parser to get the
>> AbstractSyntaxTree from method source, and if it ever fails ends up by
>> trying to decompile the byteCodes.
>>
>> This is the easy part. Now we to deal with #abstractPCForConcretePC:
>> and #abstractSourceMap.
>>
>> By reading CompiledMethod>>abstractPCForConcretePC: you should quickly
>> understand that a concrete PC is a byte offset of current byteCode
>> (the offsets displayed in the byteCode view) while the abstractPC is
>> just the rank of current byteCode in the list of byteCodes
>> instructions composing the CompiledMethod. This is just because
>> "byteCodes" may spread on several bytes beside their name...
>> This will use InstructionStream and InstructionClient which are just
>> an iterator and a sort of visitor on byteCode instructions.
>> So this is not really interesting.
>>
>> The more interesting part is #abstractSourceMap
>> There is a first step to obtain CompiledMethod>>rawSourceRangesAndMethodDo:
>> This is the most important part.
>> The rest is again a mapping from concretePC (instruction byte offset)
>> to abstractPC (instruction rank).
>> And some build of a dictionary mapping instruction rank (abstractPC)
>> -> selected range.
>>
>> Note that the last trick seems to use a regenerated CompiledMethod
>> (theMethodToScan) rather than the original CompiledMethod. There is no
>> assertion whether these two are equivalent or not. A priori, they
>> should, unless the Compiler changed since last compilation or if its
>> behaviour is affected by some Preferences... Would we introduce some
>> customizable Compiler optimizations that this could become a problem
>> (We would then add to map decompiled AST to source code AST, probably
>> with guesses, unless the CompiledMethod would contain debugger
>> friendly hints...)
>> We will consider this is not a problem by now.
>>
>> So let's now concentrate on rawSourceRangesAndMethodDo:
>> The nice thing is that you now can just debug this
>>
>> (ClosureTests>>#testToDoOutsideTemp) methodNode
>> rawSourceRangesAndMethodDo: [:rs :mth | ]
>>
>> and see how it goes in Squeak. I did not look in Pharo yet, but I
>> would be amazed to see it much different.
>> It's now late, and my spare time is off, but you have clues to get
>> more insights. I wish you good debugging, and come back to me if it
>> ever goes in deeper complications.
>>
>> Cheers
>>
>> Nicolas
>>
>> 2011/8/24 Michael Roberts <[hidden email]>:
>>>
>>>>
>>>> Ok I'm curious to know then.
>>>
>>> Here is a little trace from this example method:
>>>
>>> toDoOutsideTemp
>>> | temp collection |
>>> collection := OrderedCollection new.
>>> 1 to: 5 do: [ :index |
>>> temp := index.
>>> collection add: [ temp ] ]
>>>
>>> Trace is start,stop position of the highlight for each 'step over'.
>>>
>>> Whilst the numbers are hard to visualise, below you can see how they
>>> slightly diverge.
>>> Left Pharo  Right  Squeak
>>>
>>> 50, 73     71, 73       diff
>>> 71, 73     71, 73
>>> 50, 73     50, 73
>>> 108, 115   79, 121      diff
>>> 79, 121    79, 121
>>> 108, 115   108, 115
>>> 132, 144   132, 144
>>> 147, 146   146, 146     (diff negative size means no highlight)
>>> 146, 146   146, 146
>>> 79, 121    79, 121
>>> 108, 115   108, 115
>>> 132, 144   132, 144
>>> 147, 146   146, 146
>>> 146, 146   146, 146
>>> 79, 121    79, 121
>>> 108, 115   108, 115
>>> 132, 144   132, 144
>>> 147, 146   146, 146
>>> 146, 146   146, 146
>>> 79, 121    79, 121
>>> 108, 115   108, 115
>>> etc...
>>> For example the first difference is because Pharo shows the whole assignment
>>> of the first line as the first send, even though it is not.
>>> The second difference is that Pharo shows the assignment inside the block as
>>> the first highlight of the loop even though the to:do should be
>>> highlighted....but both Pharo & Squeak get the to:do: wrong when they choose
>>> to show it.
>>> hope you get the idea...
>>> Mike
>>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Nicolas Cellier
And by changing last line of this piece of code in #transformToDo:
       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).
 I get a "correct" selection...

Nicolas


2011/8/25 Nicolas Cellier <[hidden email]>:

> A few more notes:
>
> - Encoder>>rawSourceRanges answer a mapping AST Node -> sourceRange
>  Gnerally, I would expect sourceRange to be an Interval, but this has
> to be confirmed.
>  Theses ranges are constructed at source code Parse time (see senders
> of #noteSourceRange:forNode:)
> - The program counters are assigned to AST nodes at byte code #generate time
> - for inlined macros, like #to:do: in our case, this pc is after the
> initialize and test statements.
>
> in CompiledMethod>>#rawSourceRangesAndMethodDo:
> I just evaluated this:
>
> methNode encoder rawSourceRanges collect: [:ival |
>        sourceText copyFrom: ival first to: ival last]
>
> And what I see is that the original to:do: message (before macros
> inlining) has the right range
> {1
>        to: 5
>        do: [:index |
>                temp := index.
>                collection
>                        add: [temp]]}->a Text for 'to: 5 do: [ :index |
>                temp := index.
>                collection add: [ temp ] ]'
> This node is the original #to:do: and has pc=42
>
> There is also
> {a LeafNode}->a Text for '[ :index |
>                temp := index.
>                collection add: [ temp ] ]'
> which has a nil pc so it won't be taken into account in the source map.
>
> But one of the messages produced by the inlining has this curious range:
> {index <= 5}->a Text for 'to: 5 do: [ :index |
>                temp := index.
>                c'
> This node has pc = 40, so it will be selected before the correct one
> above has a chance to be.
> {index <= 5} is the test statement produced by inlining, and this
> seems to be the incorrect highlighting we see.
> Thus we know we have to concentrate on macro inlining.
> This happens in MessageNode>>transformToDo:
> We'll see this.
>
> In the interim, I played with eliminating the nodes having unitilialized pc
> Let us try to evaluate this snippet in debugger's inspector:
> (methNode encoder rawSourceRanges keys select: [:e | e pc > 0])
> collect: [:node |
>        | ival |
>        ival := methNode encoder rawSourceRanges at: node.
>        node -> (sourceText copyFrom: ival first to: ival last)]
>
> Oh god, a bug in the debugger:
> DoItIn: homeContext
>        ^ ((homeContext namedTempAt: 2) encoder rawSourceRanges keys
>                select: [:e | e pc > 0])
>                collect: [:node |
>                        | ival |
>                        ival := (node namedTempAt: 2) encoder rawSourceRanges at: node.
>                        node
>                                -> (sourceText copyFrom: ival first to: ival last)]
> (node namedTempAt: 2) does not mean a thing...
> A confusion occurred between the homeContext (DoItIn: method argument)
> and node (the block argument)
> Nevermind... forget about it.
>
> Now let's just concentrate on MessageNode>>transformToDo:
> We see this code:
>        test := MessageNode new
>                                receiver: blockVar
>                                selector: (increment key > 0 ifTrue: [#<=] ifFalse: [#>=])
>                                arguments: (Array with: limit)
>                                precedence: precedence from: encoder
>                                sourceRange: (myRange first to: blockRange first).
>
> So the intention seems to select 'to: 5 do: '
>
> But we see this:
> BlockNode>>noteSourceRangeStart:end:encoder:
>        "Note two source ranges for this node.  One is for the debugger
>         and is of the last expression, the result of the block.  One is for
>         source analysis and is for the entire block."
>        encoder
>                noteSourceRange: (start to: end)
>                forNode: self closureCreationNode.
>        startOfLastStatement
>                ifNil:
>                        [encoder
>                                noteSourceRange: (start to: end)
>                                forNode: self]
>                ifNotNil:
>                        [encoder
>                                noteSourceRange: (startOfLastStatement to: end - 1)
>                                forNode: self]
>
> So it seems intentional to select only the last instruction of the
> block for the debugger.
> We'd better not change this without prior asking Eliot.
> But obviously, this is not what is expected by the #transformToDo:
>
> Nicolas
>
> 2011/8/25 Stéphane Ducasse <[hidden email]>:
>> tx nicolas this is a cool way to help :)
>>
>> Stef
>>
>> On Aug 24, 2011, at 9:54 PM, Nicolas Cellier wrote:
>>
>>> So the entries of interest for highlighting are
>>>
>>> Debugger>>contentsSelection
>>> Debugger>>pcRange
>>> CompiledMethod>>debuggerMap
>>> DebuggerMethodMap class>>forMethod:
>>> DebuggerMethodMap>>rangeForPC:contextIsActiveContext:
>>>
>>> Then you see the DebuggerMethodMap>>forMethod:methodNode: takes both a
>>> CompiledMethod and its #methodNode.
>>> CompiledMethod>>methodNode invokes the Parser to get the
>>> AbstractSyntaxTree from method source, and if it ever fails ends up by
>>> trying to decompile the byteCodes.
>>>
>>> This is the easy part. Now we to deal with #abstractPCForConcretePC:
>>> and #abstractSourceMap.
>>>
>>> By reading CompiledMethod>>abstractPCForConcretePC: you should quickly
>>> understand that a concrete PC is a byte offset of current byteCode
>>> (the offsets displayed in the byteCode view) while the abstractPC is
>>> just the rank of current byteCode in the list of byteCodes
>>> instructions composing the CompiledMethod. This is just because
>>> "byteCodes" may spread on several bytes beside their name...
>>> This will use InstructionStream and InstructionClient which are just
>>> an iterator and a sort of visitor on byteCode instructions.
>>> So this is not really interesting.
>>>
>>> The more interesting part is #abstractSourceMap
>>> There is a first step to obtain CompiledMethod>>rawSourceRangesAndMethodDo:
>>> This is the most important part.
>>> The rest is again a mapping from concretePC (instruction byte offset)
>>> to abstractPC (instruction rank).
>>> And some build of a dictionary mapping instruction rank (abstractPC)
>>> -> selected range.
>>>
>>> Note that the last trick seems to use a regenerated CompiledMethod
>>> (theMethodToScan) rather than the original CompiledMethod. There is no
>>> assertion whether these two are equivalent or not. A priori, they
>>> should, unless the Compiler changed since last compilation or if its
>>> behaviour is affected by some Preferences... Would we introduce some
>>> customizable Compiler optimizations that this could become a problem
>>> (We would then add to map decompiled AST to source code AST, probably
>>> with guesses, unless the CompiledMethod would contain debugger
>>> friendly hints...)
>>> We will consider this is not a problem by now.
>>>
>>> So let's now concentrate on rawSourceRangesAndMethodDo:
>>> The nice thing is that you now can just debug this
>>>
>>> (ClosureTests>>#testToDoOutsideTemp) methodNode
>>> rawSourceRangesAndMethodDo: [:rs :mth | ]
>>>
>>> and see how it goes in Squeak. I did not look in Pharo yet, but I
>>> would be amazed to see it much different.
>>> It's now late, and my spare time is off, but you have clues to get
>>> more insights. I wish you good debugging, and come back to me if it
>>> ever goes in deeper complications.
>>>
>>> Cheers
>>>
>>> Nicolas
>>>
>>> 2011/8/24 Michael Roberts <[hidden email]>:
>>>>
>>>>>
>>>>> Ok I'm curious to know then.
>>>>
>>>> Here is a little trace from this example method:
>>>>
>>>> toDoOutsideTemp
>>>> | temp collection |
>>>> collection := OrderedCollection new.
>>>> 1 to: 5 do: [ :index |
>>>> temp := index.
>>>> collection add: [ temp ] ]
>>>>
>>>> Trace is start,stop position of the highlight for each 'step over'.
>>>>
>>>> Whilst the numbers are hard to visualise, below you can see how they
>>>> slightly diverge.
>>>> Left Pharo  Right  Squeak
>>>>
>>>> 50, 73     71, 73       diff
>>>> 71, 73     71, 73
>>>> 50, 73     50, 73
>>>> 108, 115   79, 121      diff
>>>> 79, 121    79, 121
>>>> 108, 115   108, 115
>>>> 132, 144   132, 144
>>>> 147, 146   146, 146     (diff negative size means no highlight)
>>>> 146, 146   146, 146
>>>> 79, 121    79, 121
>>>> 108, 115   108, 115
>>>> 132, 144   132, 144
>>>> 147, 146   146, 146
>>>> 146, 146   146, 146
>>>> 79, 121    79, 121
>>>> 108, 115   108, 115
>>>> 132, 144   132, 144
>>>> 147, 146   146, 146
>>>> 146, 146   146, 146
>>>> 79, 121    79, 121
>>>> 108, 115   108, 115
>>>> etc...
>>>> For example the first difference is because Pharo shows the whole assignment
>>>> of the first line as the first send, even though it is not.
>>>> The second difference is that Pharo shows the assignment inside the block as
>>>> the first highlight of the loop even though the to:do should be
>>>> highlighted....but both Pharo & Squeak get the to:do: wrong when they choose
>>>> to show it.
>>>> hope you get the idea...
>>>> Mike
>>>
>>
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Stéphane Ducasse
thanks a lot nicolas
It seems to work on my image too. I adding it to the bug entry.
What is great is that tomorrow I will be able to follow your path in the train.

Stef
On Aug 25, 2011, at 9:12 PM, Nicolas Cellier wrote:

> And by changing last line of this piece of code in #transformToDo:
>       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).
> I get a "correct" selection...
>
> Nicolas
>
>
> 2011/8/25 Nicolas Cellier <[hidden email]>:
>> A few more notes:
>>
>> - Encoder>>rawSourceRanges answer a mapping AST Node -> sourceRange
>>  Gnerally, I would expect sourceRange to be an Interval, but this has
>> to be confirmed.
>>  Theses ranges are constructed at source code Parse time (see senders
>> of #noteSourceRange:forNode:)
>> - The program counters are assigned to AST nodes at byte code #generate time
>> - for inlined macros, like #to:do: in our case, this pc is after the
>> initialize and test statements.
>>
>> in CompiledMethod>>#rawSourceRangesAndMethodDo:
>> I just evaluated this:
>>
>> methNode encoder rawSourceRanges collect: [:ival |
>>        sourceText copyFrom: ival first to: ival last]
>>
>> And what I see is that the original to:do: message (before macros
>> inlining) has the right range
>> {1
>>        to: 5
>>        do: [:index |
>>                temp := index.
>>                collection
>>                        add: [temp]]}->a Text for 'to: 5 do: [ :index |
>>                temp := index.
>>                collection add: [ temp ] ]'
>> This node is the original #to:do: and has pc=42
>>
>> There is also
>> {a LeafNode}->a Text for '[ :index |
>>                temp := index.
>>                collection add: [ temp ] ]'
>> which has a nil pc so it won't be taken into account in the source map.
>>
>> But one of the messages produced by the inlining has this curious range:
>> {index <= 5}->a Text for 'to: 5 do: [ :index |
>>                temp := index.
>>                c'
>> This node has pc = 40, so it will be selected before the correct one
>> above has a chance to be.
>> {index <= 5} is the test statement produced by inlining, and this
>> seems to be the incorrect highlighting we see.
>> Thus we know we have to concentrate on macro inlining.
>> This happens in MessageNode>>transformToDo:
>> We'll see this.
>>
>> In the interim, I played with eliminating the nodes having unitilialized pc
>> Let us try to evaluate this snippet in debugger's inspector:
>> (methNode encoder rawSourceRanges keys select: [:e | e pc > 0])
>> collect: [:node |
>>        | ival |
>>        ival := methNode encoder rawSourceRanges at: node.
>>        node -> (sourceText copyFrom: ival first to: ival last)]
>>
>> Oh god, a bug in the debugger:
>> DoItIn: homeContext
>>        ^ ((homeContext namedTempAt: 2) encoder rawSourceRanges keys
>>                select: [:e | e pc > 0])
>>                collect: [:node |
>>                        | ival |
>>                        ival := (node namedTempAt: 2) encoder rawSourceRanges at: node.
>>                        node
>>                                -> (sourceText copyFrom: ival first to: ival last)]
>> (node namedTempAt: 2) does not mean a thing...
>> A confusion occurred between the homeContext (DoItIn: method argument)
>> and node (the block argument)
>> Nevermind... forget about it.
>>
>> Now let's just concentrate on MessageNode>>transformToDo:
>> We see this code:
>>        test := MessageNode new
>>                                receiver: blockVar
>>                                selector: (increment key > 0 ifTrue: [#<=] ifFalse: [#>=])
>>                                arguments: (Array with: limit)
>>                                precedence: precedence from: encoder
>>                                sourceRange: (myRange first to: blockRange first).
>>
>> So the intention seems to select 'to: 5 do: '
>>
>> But we see this:
>> BlockNode>>noteSourceRangeStart:end:encoder:
>>        "Note two source ranges for this node.  One is for the debugger
>>         and is of the last expression, the result of the block.  One is for
>>         source analysis and is for the entire block."
>>        encoder
>>                noteSourceRange: (start to: end)
>>                forNode: self closureCreationNode.
>>        startOfLastStatement
>>                ifNil:
>>                        [encoder
>>                                noteSourceRange: (start to: end)
>>                                forNode: self]
>>                ifNotNil:
>>                        [encoder
>>                                noteSourceRange: (startOfLastStatement to: end - 1)
>>                                forNode: self]
>>
>> So it seems intentional to select only the last instruction of the
>> block for the debugger.
>> We'd better not change this without prior asking Eliot.
>> But obviously, this is not what is expected by the #transformToDo:
>>
>> Nicolas
>>
>> 2011/8/25 Stéphane Ducasse <[hidden email]>:
>>> tx nicolas this is a cool way to help :)
>>>
>>> Stef
>>>
>>> On Aug 24, 2011, at 9:54 PM, Nicolas Cellier wrote:
>>>
>>>> So the entries of interest for highlighting are
>>>>
>>>> Debugger>>contentsSelection
>>>> Debugger>>pcRange
>>>> CompiledMethod>>debuggerMap
>>>> DebuggerMethodMap class>>forMethod:
>>>> DebuggerMethodMap>>rangeForPC:contextIsActiveContext:
>>>>
>>>> Then you see the DebuggerMethodMap>>forMethod:methodNode: takes both a
>>>> CompiledMethod and its #methodNode.
>>>> CompiledMethod>>methodNode invokes the Parser to get the
>>>> AbstractSyntaxTree from method source, and if it ever fails ends up by
>>>> trying to decompile the byteCodes.
>>>>
>>>> This is the easy part. Now we to deal with #abstractPCForConcretePC:
>>>> and #abstractSourceMap.
>>>>
>>>> By reading CompiledMethod>>abstractPCForConcretePC: you should quickly
>>>> understand that a concrete PC is a byte offset of current byteCode
>>>> (the offsets displayed in the byteCode view) while the abstractPC is
>>>> just the rank of current byteCode in the list of byteCodes
>>>> instructions composing the CompiledMethod. This is just because
>>>> "byteCodes" may spread on several bytes beside their name...
>>>> This will use InstructionStream and InstructionClient which are just
>>>> an iterator and a sort of visitor on byteCode instructions.
>>>> So this is not really interesting.
>>>>
>>>> The more interesting part is #abstractSourceMap
>>>> There is a first step to obtain CompiledMethod>>rawSourceRangesAndMethodDo:
>>>> This is the most important part.
>>>> The rest is again a mapping from concretePC (instruction byte offset)
>>>> to abstractPC (instruction rank).
>>>> And some build of a dictionary mapping instruction rank (abstractPC)
>>>> -> selected range.
>>>>
>>>> Note that the last trick seems to use a regenerated CompiledMethod
>>>> (theMethodToScan) rather than the original CompiledMethod. There is no
>>>> assertion whether these two are equivalent or not. A priori, they
>>>> should, unless the Compiler changed since last compilation or if its
>>>> behaviour is affected by some Preferences... Would we introduce some
>>>> customizable Compiler optimizations that this could become a problem
>>>> (We would then add to map decompiled AST to source code AST, probably
>>>> with guesses, unless the CompiledMethod would contain debugger
>>>> friendly hints...)
>>>> We will consider this is not a problem by now.
>>>>
>>>> So let's now concentrate on rawSourceRangesAndMethodDo:
>>>> The nice thing is that you now can just debug this
>>>>
>>>> (ClosureTests>>#testToDoOutsideTemp) methodNode
>>>> rawSourceRangesAndMethodDo: [:rs :mth | ]
>>>>
>>>> and see how it goes in Squeak. I did not look in Pharo yet, but I
>>>> would be amazed to see it much different.
>>>> It's now late, and my spare time is off, but you have clues to get
>>>> more insights. I wish you good debugging, and come back to me if it
>>>> ever goes in deeper complications.
>>>>
>>>> Cheers
>>>>
>>>> Nicolas
>>>>
>>>> 2011/8/24 Michael Roberts <[hidden email]>:
>>>>>
>>>>>>
>>>>>> Ok I'm curious to know then.
>>>>>
>>>>> Here is a little trace from this example method:
>>>>>
>>>>> toDoOutsideTemp
>>>>> | temp collection |
>>>>> collection := OrderedCollection new.
>>>>> 1 to: 5 do: [ :index |
>>>>> temp := index.
>>>>> collection add: [ temp ] ]
>>>>>
>>>>> Trace is start,stop position of the highlight for each 'step over'.
>>>>>
>>>>> Whilst the numbers are hard to visualise, below you can see how they
>>>>> slightly diverge.
>>>>> Left Pharo  Right  Squeak
>>>>>
>>>>> 50, 73     71, 73       diff
>>>>> 71, 73     71, 73
>>>>> 50, 73     50, 73
>>>>> 108, 115   79, 121      diff
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> 132, 144   132, 144
>>>>> 147, 146   146, 146     (diff negative size means no highlight)
>>>>> 146, 146   146, 146
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> 132, 144   132, 144
>>>>> 147, 146   146, 146
>>>>> 146, 146   146, 146
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> 132, 144   132, 144
>>>>> 147, 146   146, 146
>>>>> 146, 146   146, 146
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> etc...
>>>>> For example the first difference is because Pharo shows the whole assignment
>>>>> of the first line as the first send, even though it is not.
>>>>> The second difference is that Pharo shows the assignment inside the block as
>>>>> the first highlight of the loop even though the to:do should be
>>>>> highlighted....but both Pharo & Squeak get the to:do: wrong when they choose
>>>>> to show it.
>>>>> hope you get the idea...
>>>>> Mike
>>>>
>>>
>>>
>>>
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Michael Roberts-2
I will try it in my image, thanks!

I got a little further with Andres. The thing about the loop example is that the block is copying values outside its scope. You get get extra bytecodes at the start of the method (Array new: ...) and extra bytecodes in the loop to do the copying of values. We suspect that the mapping is getting confused by this. This is because the encoder hands out the ranges of the original method correctly, but the mapping has to align the pc to ignore these bytecodes because they are not in the debugger source. Anyway that was our impression. I agree we need Eliot to review.

cheers,
Mike

On Thu, Aug 25, 2011 at 9:43 PM, Stéphane Ducasse <[hidden email]> wrote:
thanks a lot nicolas
It seems to work on my image too. I adding it to the bug entry.
What is great is that tomorrow I will be able to follow your path in the train.

Stef
On Aug 25, 2011, at 9:12 PM, Nicolas Cellier wrote:

> And by changing last line of this piece of code in #transformToDo:
>       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).
> I get a "correct" selection...
>
> Nicolas
>
>
> 2011/8/25 Nicolas Cellier <[hidden email]>:
>> A few more notes:
>>
>> - Encoder>>rawSourceRanges answer a mapping AST Node -> sourceRange
>>  Gnerally, I would expect sourceRange to be an Interval, but this has
>> to be confirmed.
>>  Theses ranges are constructed at source code Parse time (see senders
>> of #noteSourceRange:forNode:)
>> - The program counters are assigned to AST nodes at byte code #generate time
>> - for inlined macros, like #to:do: in our case, this pc is after the
>> initialize and test statements.
>>
>> in CompiledMethod>>#rawSourceRangesAndMethodDo:
>> I just evaluated this:
>>
>> methNode encoder rawSourceRanges collect: [:ival |
>>        sourceText copyFrom: ival first to: ival last]
>>
>> And what I see is that the original to:do: message (before macros
>> inlining) has the right range
>> {1
>>        to: 5
>>        do: [:index |
>>                temp := index.
>>                collection
>>                        add: [temp]]}->a Text for 'to: 5 do: [ :index |
>>                temp := index.
>>                collection add: [ temp ] ]'
>> This node is the original #to:do: and has pc=42
>>
>> There is also
>> {a LeafNode}->a Text for '[ :index |
>>                temp := index.
>>                collection add: [ temp ] ]'
>> which has a nil pc so it won't be taken into account in the source map.
>>
>> But one of the messages produced by the inlining has this curious range:
>> {index <= 5}->a Text for 'to: 5 do: [ :index |
>>                temp := index.
>>                c'
>> This node has pc = 40, so it will be selected before the correct one
>> above has a chance to be.
>> {index <= 5} is the test statement produced by inlining, and this
>> seems to be the incorrect highlighting we see.
>> Thus we know we have to concentrate on macro inlining.
>> This happens in MessageNode>>transformToDo:
>> We'll see this.
>>
>> In the interim, I played with eliminating the nodes having unitilialized pc
>> Let us try to evaluate this snippet in debugger's inspector:
>> (methNode encoder rawSourceRanges keys select: [:e | e pc > 0])
>> collect: [:node |
>>        | ival |
>>        ival := methNode encoder rawSourceRanges at: node.
>>        node -> (sourceText copyFrom: ival first to: ival last)]
>>
>> Oh god, a bug in the debugger:
>> DoItIn: homeContext
>>        ^ ((homeContext namedTempAt: 2) encoder rawSourceRanges keys
>>                select: [:e | e pc > 0])
>>                collect: [:node |
>>                        | ival |
>>                        ival := (node namedTempAt: 2) encoder rawSourceRanges at: node.
>>                        node
>>                                -> (sourceText copyFrom: ival first to: ival last)]
>> (node namedTempAt: 2) does not mean a thing...
>> A confusion occurred between the homeContext (DoItIn: method argument)
>> and node (the block argument)
>> Nevermind... forget about it.
>>
>> Now let's just concentrate on MessageNode>>transformToDo:
>> We see this code:
>>        test := MessageNode new
>>                                receiver: blockVar
>>                                selector: (increment key > 0 ifTrue: [#<=] ifFalse: [#>=])
>>                                arguments: (Array with: limit)
>>                                precedence: precedence from: encoder
>>                                sourceRange: (myRange first to: blockRange first).
>>
>> So the intention seems to select 'to: 5 do: '
>>
>> But we see this:
>> BlockNode>>noteSourceRangeStart:end:encoder:
>>        "Note two source ranges for this node.  One is for the debugger
>>         and is of the last expression, the result of the block.  One is for
>>         source analysis and is for the entire block."
>>        encoder
>>                noteSourceRange: (start to: end)
>>                forNode: self closureCreationNode.
>>        startOfLastStatement
>>                ifNil:
>>                        [encoder
>>                                noteSourceRange: (start to: end)
>>                                forNode: self]
>>                ifNotNil:
>>                        [encoder
>>                                noteSourceRange: (startOfLastStatement to: end - 1)
>>                                forNode: self]
>>
>> So it seems intentional to select only the last instruction of the
>> block for the debugger.
>> We'd better not change this without prior asking Eliot.
>> But obviously, this is not what is expected by the #transformToDo:
>>
>> Nicolas
>>
>> 2011/8/25 Stéphane Ducasse <[hidden email]>:
>>> tx nicolas this is a cool way to help :)
>>>
>>> Stef
>>>
>>> On Aug 24, 2011, at 9:54 PM, Nicolas Cellier wrote:
>>>
>>>> So the entries of interest for highlighting are
>>>>
>>>> Debugger>>contentsSelection
>>>> Debugger>>pcRange
>>>> CompiledMethod>>debuggerMap
>>>> DebuggerMethodMap class>>forMethod:
>>>> DebuggerMethodMap>>rangeForPC:contextIsActiveContext:
>>>>
>>>> Then you see the DebuggerMethodMap>>forMethod:methodNode: takes both a
>>>> CompiledMethod and its #methodNode.
>>>> CompiledMethod>>methodNode invokes the Parser to get the
>>>> AbstractSyntaxTree from method source, and if it ever fails ends up by
>>>> trying to decompile the byteCodes.
>>>>
>>>> This is the easy part. Now we to deal with #abstractPCForConcretePC:
>>>> and #abstractSourceMap.
>>>>
>>>> By reading CompiledMethod>>abstractPCForConcretePC: you should quickly
>>>> understand that a concrete PC is a byte offset of current byteCode
>>>> (the offsets displayed in the byteCode view) while the abstractPC is
>>>> just the rank of current byteCode in the list of byteCodes
>>>> instructions composing the CompiledMethod. This is just because
>>>> "byteCodes" may spread on several bytes beside their name...
>>>> This will use InstructionStream and InstructionClient which are just
>>>> an iterator and a sort of visitor on byteCode instructions.
>>>> So this is not really interesting.
>>>>
>>>> The more interesting part is #abstractSourceMap
>>>> There is a first step to obtain CompiledMethod>>rawSourceRangesAndMethodDo:
>>>> This is the most important part.
>>>> The rest is again a mapping from concretePC (instruction byte offset)
>>>> to abstractPC (instruction rank).
>>>> And some build of a dictionary mapping instruction rank (abstractPC)
>>>> -> selected range.
>>>>
>>>> Note that the last trick seems to use a regenerated CompiledMethod
>>>> (theMethodToScan) rather than the original CompiledMethod. There is no
>>>> assertion whether these two are equivalent or not. A priori, they
>>>> should, unless the Compiler changed since last compilation or if its
>>>> behaviour is affected by some Preferences... Would we introduce some
>>>> customizable Compiler optimizations that this could become a problem
>>>> (We would then add to map decompiled AST to source code AST, probably
>>>> with guesses, unless the CompiledMethod would contain debugger
>>>> friendly hints...)
>>>> We will consider this is not a problem by now.
>>>>
>>>> So let's now concentrate on rawSourceRangesAndMethodDo:
>>>> The nice thing is that you now can just debug this
>>>>
>>>> (ClosureTests>>#testToDoOutsideTemp) methodNode
>>>> rawSourceRangesAndMethodDo: [:rs :mth | ]
>>>>
>>>> and see how it goes in Squeak. I did not look in Pharo yet, but I
>>>> would be amazed to see it much different.
>>>> It's now late, and my spare time is off, but you have clues to get
>>>> more insights. I wish you good debugging, and come back to me if it
>>>> ever goes in deeper complications.
>>>>
>>>> Cheers
>>>>
>>>> Nicolas
>>>>
>>>> 2011/8/24 Michael Roberts <[hidden email]>:
>>>>>
>>>>>>
>>>>>> Ok I'm curious to know then.
>>>>>
>>>>> Here is a little trace from this example method:
>>>>>
>>>>> toDoOutsideTemp
>>>>> | temp collection |
>>>>> collection := OrderedCollection new.
>>>>> 1 to: 5 do: [ :index |
>>>>> temp := index.
>>>>> collection add: [ temp ] ]
>>>>>
>>>>> Trace is start,stop position of the highlight for each 'step over'.
>>>>>
>>>>> Whilst the numbers are hard to visualise, below you can see how they
>>>>> slightly diverge.
>>>>> Left Pharo  Right  Squeak
>>>>>
>>>>> 50, 73     71, 73       diff
>>>>> 71, 73     71, 73
>>>>> 50, 73     50, 73
>>>>> 108, 115   79, 121      diff
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> 132, 144   132, 144
>>>>> 147, 146   146, 146     (diff negative size means no highlight)
>>>>> 146, 146   146, 146
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> 132, 144   132, 144
>>>>> 147, 146   146, 146
>>>>> 146, 146   146, 146
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> 132, 144   132, 144
>>>>> 147, 146   146, 146
>>>>> 146, 146   146, 146
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> etc...
>>>>> For example the first difference is because Pharo shows the whole assignment
>>>>> of the first line as the first send, even though it is not.
>>>>> The second difference is that Pharo shows the assignment inside the block as
>>>>> the first highlight of the loop even though the to:do should be
>>>>> highlighted....but both Pharo & Squeak get the to:do: wrong when they choose
>>>>> to show it.
>>>>> hope you get the idea...
>>>>> Mike
>>>>
>>>
>>>
>>>
>>
>



Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Michael Roberts-2
Hi the loop looks much better, but the first assignment (which was not in the original TestCase btw, it is just a side effect of the way i wrote my test) is still wrong. I think this is because it needs to start at pc 3 (?) rather than 2.

I have attached my little analyser to the issue.  I will post it in task forces or somewhere when it is cleaned a little.

Usage

DebuggerAnalyser inspectOn: #toDoOutsideTemp.

This opens a debugger and an inspector. It positions the debugger at the method under 'analysis'. At the moment the methods are hard coded in the class itself, but it would not be hard to be more general. There is also the code in there to do this for arbitrary doits, but i have not wired this up yet. This class was originally a large script.....

You can also do a trace:

DebuggerAnalyser new traceSelector: #toDoOutsideTemp .

I plan to use this class eventually for regression testing the debugger. So we will run tests that trace exact highlight sequences in Jenkins.

Also there is a little text morph for visualising highlight intervals

self openAnalysisText

and then you can poke highlights via analysisHighlight: (3 to: 6)

If you have the analyser open in one area, and then do things like

(DebuggerAnalyser>>#basicAssignment) methodNode rawSourceRanges.

in a separate debugger, then you can get the highlight points and 'visualise' what is going on at various stages of the machinery. Hope you get the idea, if this is useful...

cheers,
Mike


On Fri, Aug 26, 2011 at 9:00 AM, Michael Roberts <[hidden email]> wrote:
I will try it in my image, thanks!

I got a little further with Andres. The thing about the loop example is that the block is copying values outside its scope. You get get extra bytecodes at the start of the method (Array new: ...) and extra bytecodes in the loop to do the copying of values. We suspect that the mapping is getting confused by this. This is because the encoder hands out the ranges of the original method correctly, but the mapping has to align the pc to ignore these bytecodes because they are not in the debugger source. Anyway that was our impression. I agree we need Eliot to review.

cheers,
Mike


On Thu, Aug 25, 2011 at 9:43 PM, Stéphane Ducasse <[hidden email]> wrote:
thanks a lot nicolas
It seems to work on my image too. I adding it to the bug entry.
What is great is that tomorrow I will be able to follow your path in the train.

Stef
On Aug 25, 2011, at 9:12 PM, Nicolas Cellier wrote:

> And by changing last line of this piece of code in #transformToDo:
>       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).
> I get a "correct" selection...
>
> Nicolas
>
>
> 2011/8/25 Nicolas Cellier <[hidden email]>:
>> A few more notes:
>>
>> - Encoder>>rawSourceRanges answer a mapping AST Node -> sourceRange
>>  Gnerally, I would expect sourceRange to be an Interval, but this has
>> to be confirmed.
>>  Theses ranges are constructed at source code Parse time (see senders
>> of #noteSourceRange:forNode:)
>> - The program counters are assigned to AST nodes at byte code #generate time
>> - for inlined macros, like #to:do: in our case, this pc is after the
>> initialize and test statements.
>>
>> in CompiledMethod>>#rawSourceRangesAndMethodDo:
>> I just evaluated this:
>>
>> methNode encoder rawSourceRanges collect: [:ival |
>>        sourceText copyFrom: ival first to: ival last]
>>
>> And what I see is that the original to:do: message (before macros
>> inlining) has the right range
>> {1
>>        to: 5
>>        do: [:index |
>>                temp := index.
>>                collection
>>                        add: [temp]]}->a Text for 'to: 5 do: [ :index |
>>                temp := index.
>>                collection add: [ temp ] ]'
>> This node is the original #to:do: and has pc=42
>>
>> There is also
>> {a LeafNode}->a Text for '[ :index |
>>                temp := index.
>>                collection add: [ temp ] ]'
>> which has a nil pc so it won't be taken into account in the source map.
>>
>> But one of the messages produced by the inlining has this curious range:
>> {index <= 5}->a Text for 'to: 5 do: [ :index |
>>                temp := index.
>>                c'
>> This node has pc = 40, so it will be selected before the correct one
>> above has a chance to be.
>> {index <= 5} is the test statement produced by inlining, and this
>> seems to be the incorrect highlighting we see.
>> Thus we know we have to concentrate on macro inlining.
>> This happens in MessageNode>>transformToDo:
>> We'll see this.
>>
>> In the interim, I played with eliminating the nodes having unitilialized pc
>> Let us try to evaluate this snippet in debugger's inspector:
>> (methNode encoder rawSourceRanges keys select: [:e | e pc > 0])
>> collect: [:node |
>>        | ival |
>>        ival := methNode encoder rawSourceRanges at: node.
>>        node -> (sourceText copyFrom: ival first to: ival last)]
>>
>> Oh god, a bug in the debugger:
>> DoItIn: homeContext
>>        ^ ((homeContext namedTempAt: 2) encoder rawSourceRanges keys
>>                select: [:e | e pc > 0])
>>                collect: [:node |
>>                        | ival |
>>                        ival := (node namedTempAt: 2) encoder rawSourceRanges at: node.
>>                        node
>>                                -> (sourceText copyFrom: ival first to: ival last)]
>> (node namedTempAt: 2) does not mean a thing...
>> A confusion occurred between the homeContext (DoItIn: method argument)
>> and node (the block argument)
>> Nevermind... forget about it.
>>
>> Now let's just concentrate on MessageNode>>transformToDo:
>> We see this code:
>>        test := MessageNode new
>>                                receiver: blockVar
>>                                selector: (increment key > 0 ifTrue: [#<=] ifFalse: [#>=])
>>                                arguments: (Array with: limit)
>>                                precedence: precedence from: encoder
>>                                sourceRange: (myRange first to: blockRange first).
>>
>> So the intention seems to select 'to: 5 do: '
>>
>> But we see this:
>> BlockNode>>noteSourceRangeStart:end:encoder:
>>        "Note two source ranges for this node.  One is for the debugger
>>         and is of the last expression, the result of the block.  One is for
>>         source analysis and is for the entire block."
>>        encoder
>>                noteSourceRange: (start to: end)
>>                forNode: self closureCreationNode.
>>        startOfLastStatement
>>                ifNil:
>>                        [encoder
>>                                noteSourceRange: (start to: end)
>>                                forNode: self]
>>                ifNotNil:
>>                        [encoder
>>                                noteSourceRange: (startOfLastStatement to: end - 1)
>>                                forNode: self]
>>
>> So it seems intentional to select only the last instruction of the
>> block for the debugger.
>> We'd better not change this without prior asking Eliot.
>> But obviously, this is not what is expected by the #transformToDo:
>>
>> Nicolas
>>
>> 2011/8/25 Stéphane Ducasse <[hidden email]>:
>>> tx nicolas this is a cool way to help :)
>>>
>>> Stef
>>>
>>> On Aug 24, 2011, at 9:54 PM, Nicolas Cellier wrote:
>>>
>>>> So the entries of interest for highlighting are
>>>>
>>>> Debugger>>contentsSelection
>>>> Debugger>>pcRange
>>>> CompiledMethod>>debuggerMap
>>>> DebuggerMethodMap class>>forMethod:
>>>> DebuggerMethodMap>>rangeForPC:contextIsActiveContext:
>>>>
>>>> Then you see the DebuggerMethodMap>>forMethod:methodNode: takes both a
>>>> CompiledMethod and its #methodNode.
>>>> CompiledMethod>>methodNode invokes the Parser to get the
>>>> AbstractSyntaxTree from method source, and if it ever fails ends up by
>>>> trying to decompile the byteCodes.
>>>>
>>>> This is the easy part. Now we to deal with #abstractPCForConcretePC:
>>>> and #abstractSourceMap.
>>>>
>>>> By reading CompiledMethod>>abstractPCForConcretePC: you should quickly
>>>> understand that a concrete PC is a byte offset of current byteCode
>>>> (the offsets displayed in the byteCode view) while the abstractPC is
>>>> just the rank of current byteCode in the list of byteCodes
>>>> instructions composing the CompiledMethod. This is just because
>>>> "byteCodes" may spread on several bytes beside their name...
>>>> This will use InstructionStream and InstructionClient which are just
>>>> an iterator and a sort of visitor on byteCode instructions.
>>>> So this is not really interesting.
>>>>
>>>> The more interesting part is #abstractSourceMap
>>>> There is a first step to obtain CompiledMethod>>rawSourceRangesAndMethodDo:
>>>> This is the most important part.
>>>> The rest is again a mapping from concretePC (instruction byte offset)
>>>> to abstractPC (instruction rank).
>>>> And some build of a dictionary mapping instruction rank (abstractPC)
>>>> -> selected range.
>>>>
>>>> Note that the last trick seems to use a regenerated CompiledMethod
>>>> (theMethodToScan) rather than the original CompiledMethod. There is no
>>>> assertion whether these two are equivalent or not. A priori, they
>>>> should, unless the Compiler changed since last compilation or if its
>>>> behaviour is affected by some Preferences... Would we introduce some
>>>> customizable Compiler optimizations that this could become a problem
>>>> (We would then add to map decompiled AST to source code AST, probably
>>>> with guesses, unless the CompiledMethod would contain debugger
>>>> friendly hints...)
>>>> We will consider this is not a problem by now.
>>>>
>>>> So let's now concentrate on rawSourceRangesAndMethodDo:
>>>> The nice thing is that you now can just debug this
>>>>
>>>> (ClosureTests>>#testToDoOutsideTemp) methodNode
>>>> rawSourceRangesAndMethodDo: [:rs :mth | ]
>>>>
>>>> and see how it goes in Squeak. I did not look in Pharo yet, but I
>>>> would be amazed to see it much different.
>>>> It's now late, and my spare time is off, but you have clues to get
>>>> more insights. I wish you good debugging, and come back to me if it
>>>> ever goes in deeper complications.
>>>>
>>>> Cheers
>>>>
>>>> Nicolas
>>>>
>>>> 2011/8/24 Michael Roberts <[hidden email]>:
>>>>>
>>>>>>
>>>>>> Ok I'm curious to know then.
>>>>>
>>>>> Here is a little trace from this example method:
>>>>>
>>>>> toDoOutsideTemp
>>>>> | temp collection |
>>>>> collection := OrderedCollection new.
>>>>> 1 to: 5 do: [ :index |
>>>>> temp := index.
>>>>> collection add: [ temp ] ]
>>>>>
>>>>> Trace is start,stop position of the highlight for each 'step over'.
>>>>>
>>>>> Whilst the numbers are hard to visualise, below you can see how they
>>>>> slightly diverge.
>>>>> Left Pharo  Right  Squeak
>>>>>
>>>>> 50, 73     71, 73       diff
>>>>> 71, 73     71, 73
>>>>> 50, 73     50, 73
>>>>> 108, 115   79, 121      diff
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> 132, 144   132, 144
>>>>> 147, 146   146, 146     (diff negative size means no highlight)
>>>>> 146, 146   146, 146
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> 132, 144   132, 144
>>>>> 147, 146   146, 146
>>>>> 146, 146   146, 146
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> 132, 144   132, 144
>>>>> 147, 146   146, 146
>>>>> 146, 146   146, 146
>>>>> 79, 121    79, 121
>>>>> 108, 115   108, 115
>>>>> etc...
>>>>> For example the first difference is because Pharo shows the whole assignment
>>>>> of the first line as the first send, even though it is not.
>>>>> The second difference is that Pharo shows the assignment inside the block as
>>>>> the first highlight of the loop even though the to:do should be
>>>>> highlighted....but both Pharo & Squeak get the to:do: wrong when they choose
>>>>> to show it.
>>>>> hope you get the idea...
>>>>> Mike
>>>>
>>>
>>>
>>>
>>
>




Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Stéphane Ducasse
While working on a chapter on blocks I got the following problems



foo

        | a|
        a := 0.
        ^ {[a :=2] .[a]}


| res |
res := ZnClientTests new foo.
res second value crLog.
res first value.
res second value crLog.

stepping into new foo first positioned me only on the [a :=2 ] skipping the first assignment
second I get debuggers when I want to try to get information.

Stef
Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Michael Roberts-2
ok i'll take a look and try add some test cases to the analyser....

Mike

On Fri, Aug 26, 2011 at 5:16 PM, Stéphane Ducasse <[hidden email]> wrote:
While working on a chapter on blocks I got the following problems



foo

       | a|
       a := 0.
       ^ {[a :=2] .[a]}


| res |
res := ZnClientTests new foo.
res second value crLog.
res first value.
res second value crLog.

stepping into new foo first positioned me only on the [a :=2 ] skipping the first assignment
second I get debuggers when I want to try to get information.

Stef

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Stéphane Ducasse
I read the mail of nicolas in the train (amazing no cooling system so it was more like a sona and I never realize before that second class
in uk was o bad for tall people) and yes this code is complex. Having an AST based debugger is definitively an interesting alternative.

Stef

On Aug 27, 2011, at 10:58 AM, Michael Roberts wrote:

> ok i'll take a look and try add some test cases to the analyser....
>
> Mike
>
> On Fri, Aug 26, 2011 at 5:16 PM, Stéphane Ducasse <[hidden email]> wrote:
> While working on a chapter on blocks I got the following problems
>
>
>
> foo
>
>        | a|
>        a := 0.
>        ^ {[a :=2] .[a]}
>
>
> | res |
> res := ZnClientTests new foo.
> res second value crLog.
> res first value.
> res second value crLog.
>
> stepping into new foo first positioned me only on the [a :=2 ] skipping the first assignment
> second I get debuggers when I want to try to get information.
>
> Stef
>


Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Andrea Brühlmann-2
In reply to this post by Stéphane Ducasse
Hello Stef,

I appreciate the work of the pharo community! We will continue reporting bugs and submit fixes that
we made. Of course I do not expect anyone to fix all reported bugs, but there are issues like 4694
(debugger) where we really need your help. Other issues like the buggy code completion can be
workarounded by me by disabling code completion ;-)

So thanks for the welcome and I am looking forward to having 4694 fixed!

Andrea




Stéphane Ducasse schrieb:

> Thanks andrea
>
> Welcome to the pharo mailing-list and community.
>
> Now I think that this is important that inside netstyle you also consider that if pharo is important for you (which I imagine)
> that you should also contribute. Bug reporting is already a contribution.  Pharo is an open-source software mainly supported by the free time of people, people that are often do not earning their money from their pharo work (I thank them for all that).
> I also understand that people can get frustrated by changes but what can we do?
>
> May be people can gather and check the fixes that are important to fix but we do not have the force to maintain.
>
> Imagine well that we have one engineer full time since 8 months.
>
> Stef
>
>
> On Aug 23, 2011, at 8:54 AM, Andrea Brühlmann wrote:
>
>> Hello,
>>
>> I reported some bugs on the issue tracker and hope you can help me with them!
>>
>>
>> 4681: Accepting with the enter key does not work
>> http://code.google.com/p/pharo/issues/detail?id=4681
>>
>> 4682: Code completion makes strange cursor placements and too many spaces
>> http://code.google.com/p/pharo/issues/detail?id=4682
>>
>> 4683: Code completion breaks some search fields (errors during typing)
>> http://code.google.com/p/pharo/issues/detail?id=4683
>>
>> 4684: Missing ctrl+w (methodNamesContainingIt:)
>> http://code.google.com/p/pharo/issues/detail?id=4684
>>
>> 4685: Merge dialog: cannot resolve conflict with removed method
>> http://code.google.com/p/pharo/issues/detail?id=4685
>>
>> 4686: Method category change creates no new version
>> http://code.google.com/p/pharo/issues/detail?id=4686
>>
>> 4687: Errors during coding (MessageNotUnderstood: receiver of "morph" is nil)
>> http://code.google.com/p/pharo/issues/detail?id=4687
>>
>> 4688: Progress bar disappears on image save
>> http://code.google.com/p/pharo/issues/detail?id=4688
>>
>> 4689: MessageTally bug
>> http://code.google.com/p/pharo/issues/detail?id=4689
>>
>> 4690: Progress bar position
>> http://code.google.com/p/pharo/issues/detail?id=4690
>>
>> 4691: Bad line breaks in code until window is resized (because of bold text?)
>> http://code.google.com/p/pharo/issues/detail?id=4691
>>
>> 4692: Drop downs should select the default entry (topmost) instead of a middle one
>> http://code.google.com/p/pharo/issues/detail?id=4692
>>
>> 4693: Line breaks in tooltips are wrong
>> http://code.google.com/p/pharo/issues/detail?id=4693
>>
>> 4694: Debugger: stepping over an error can not open a new debugger until I hit cmd+.
>> http://code.google.com/p/pharo/issues/detail?id=4694
>>
>> 4695: Time asString prints nanos unrounded
>> http://code.google.com/p/pharo/issues/detail?id=4695
>>
>>
>> --
>> Andrea Brühlmann
>>
>
>
>


--
AB    |   ANDREA BRÜHLMANN · SOFTWARE ENGINEER
       |   NETSTYLE · TERRASSENWEG 18 · CH-3012 BERN
       |   TEL  +41 31 356 42 54 · FAX  +41 31 356 42 51
       |   WWW.NETSTYLE.CH · [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Stéphane Ducasse
Thanks for the point.
Yes it is important.

I will ask igor to have a look in two or three weeks.

Stef

On Sep 5, 2011, at 9:07 AM, Andrea Brühlmann wrote:

> Hello Stef,
>
> I appreciate the work of the pharo community! We will continue reporting bugs and submit fixes that
> we made. Of course I do not expect anyone to fix all reported bugs, but there are issues like 4694
> (debugger) where we really need your help. Other issues like the buggy code completion can be
> workarounded by me by disabling code completion ;-)
>
> So thanks for the welcome and I am looking forward to having 4694 fixed!
>
> Andrea
>
>
>
>
> Stéphane Ducasse schrieb:
>> Thanks andrea
>> Welcome to the pharo mailing-list and community. Now I think that this is important that inside netstyle you also consider that if pharo is important for you (which I imagine) that you should also contribute. Bug reporting is already a contribution.  Pharo is an open-source software mainly supported by the free time of people, people that are often do not earning their money from their pharo work (I thank them for all that). I also understand that people can get frustrated by changes but what can we do?
>> May be people can gather and check the fixes that are important to fix but we do not have the force to maintain.
>> Imagine well that we have one engineer full time since 8 months. Stef
>> On Aug 23, 2011, at 8:54 AM, Andrea Brühlmann wrote:
>>> Hello,
>>>
>>> I reported some bugs on the issue tracker and hope you can help me with them!
>>>
>>>
>>> 4681: Accepting with the enter key does not work
>>> http://code.google.com/p/pharo/issues/detail?id=4681
>>>
>>> 4682: Code completion makes strange cursor placements and too many spaces
>>> http://code.google.com/p/pharo/issues/detail?id=4682
>>>
>>> 4683: Code completion breaks some search fields (errors during typing)
>>> http://code.google.com/p/pharo/issues/detail?id=4683
>>>
>>> 4684: Missing ctrl+w (methodNamesContainingIt:)
>>> http://code.google.com/p/pharo/issues/detail?id=4684
>>>
>>> 4685: Merge dialog: cannot resolve conflict with removed method
>>> http://code.google.com/p/pharo/issues/detail?id=4685
>>>
>>> 4686: Method category change creates no new version
>>> http://code.google.com/p/pharo/issues/detail?id=4686
>>>
>>> 4687: Errors during coding (MessageNotUnderstood: receiver of "morph" is nil)
>>> http://code.google.com/p/pharo/issues/detail?id=4687
>>>
>>> 4688: Progress bar disappears on image save
>>> http://code.google.com/p/pharo/issues/detail?id=4688
>>>
>>> 4689: MessageTally bug
>>> http://code.google.com/p/pharo/issues/detail?id=4689
>>>
>>> 4690: Progress bar position
>>> http://code.google.com/p/pharo/issues/detail?id=4690
>>>
>>> 4691: Bad line breaks in code until window is resized (because of bold text?)
>>> http://code.google.com/p/pharo/issues/detail?id=4691
>>>
>>> 4692: Drop downs should select the default entry (topmost) instead of a middle one
>>> http://code.google.com/p/pharo/issues/detail?id=4692
>>>
>>> 4693: Line breaks in tooltips are wrong
>>> http://code.google.com/p/pharo/issues/detail?id=4693
>>>
>>> 4694: Debugger: stepping over an error can not open a new debugger until I hit cmd+.
>>> http://code.google.com/p/pharo/issues/detail?id=4694
>>>
>>> 4695: Time asString prints nanos unrounded
>>> http://code.google.com/p/pharo/issues/detail?id=4695
>>>
>>>
>>> --
>>> Andrea Brühlmann
>>>
>
>
> --
> AB    |   ANDREA BRÜHLMANN · SOFTWARE ENGINEER
>      |   NETSTYLE · TERRASSENWEG 18 · CH-3012 BERN
>      |   TEL  +41 31 356 42 54 · FAX  +41 31 356 42 51
>      |   WWW.NETSTYLE.CH · [hidden email]
>


Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Stéphane Ducasse
In reply to this post by Andrea Brühlmann-2
BTW mike roberts is building a tool to be able to understand and fix the code range of the debugger hilighting.
Probably your bug is also related to the closure introduction.

Now the abstraction used is so low level that this is a pain and fragile.
Ideally using an AST would be much better but this is for the future.

Stef


On Sep 5, 2011, at 9:07 AM, Andrea Brühlmann wrote:

> Hello Stef,
>
> I appreciate the work of the pharo community! We will continue reporting bugs and submit fixes that
> we made. Of course I do not expect anyone to fix all reported bugs, but there are issues like 4694
> (debugger) where we really need your help. Other issues like the buggy code completion can be
> workarounded by me by disabling code completion ;-)
>
> So thanks for the welcome and I am looking forward to having 4694 fixed!
>
> Andrea
>
>
>
>
> Stéphane Ducasse schrieb:
>> Thanks andrea
>> Welcome to the pharo mailing-list and community. Now I think that this is important that inside netstyle you also consider that if pharo is important for you (which I imagine) that you should also contribute. Bug reporting is already a contribution.  Pharo is an open-source software mainly supported by the free time of people, people that are often do not earning their money from their pharo work (I thank them for all that). I also understand that people can get frustrated by changes but what can we do?
>> May be people can gather and check the fixes that are important to fix but we do not have the force to maintain.
>> Imagine well that we have one engineer full time since 8 months. Stef
>> On Aug 23, 2011, at 8:54 AM, Andrea Brühlmann wrote:
>>> Hello,
>>>
>>> I reported some bugs on the issue tracker and hope you can help me with them!
>>>
>>>
>>> 4681: Accepting with the enter key does not work
>>> http://code.google.com/p/pharo/issues/detail?id=4681
>>>
>>> 4682: Code completion makes strange cursor placements and too many spaces
>>> http://code.google.com/p/pharo/issues/detail?id=4682
>>>
>>> 4683: Code completion breaks some search fields (errors during typing)
>>> http://code.google.com/p/pharo/issues/detail?id=4683
>>>
>>> 4684: Missing ctrl+w (methodNamesContainingIt:)
>>> http://code.google.com/p/pharo/issues/detail?id=4684
>>>
>>> 4685: Merge dialog: cannot resolve conflict with removed method
>>> http://code.google.com/p/pharo/issues/detail?id=4685
>>>
>>> 4686: Method category change creates no new version
>>> http://code.google.com/p/pharo/issues/detail?id=4686
>>>
>>> 4687: Errors during coding (MessageNotUnderstood: receiver of "morph" is nil)
>>> http://code.google.com/p/pharo/issues/detail?id=4687
>>>
>>> 4688: Progress bar disappears on image save
>>> http://code.google.com/p/pharo/issues/detail?id=4688
>>>
>>> 4689: MessageTally bug
>>> http://code.google.com/p/pharo/issues/detail?id=4689
>>>
>>> 4690: Progress bar position
>>> http://code.google.com/p/pharo/issues/detail?id=4690
>>>
>>> 4691: Bad line breaks in code until window is resized (because of bold text?)
>>> http://code.google.com/p/pharo/issues/detail?id=4691
>>>
>>> 4692: Drop downs should select the default entry (topmost) instead of a middle one
>>> http://code.google.com/p/pharo/issues/detail?id=4692
>>>
>>> 4693: Line breaks in tooltips are wrong
>>> http://code.google.com/p/pharo/issues/detail?id=4693
>>>
>>> 4694: Debugger: stepping over an error can not open a new debugger until I hit cmd+.
>>> http://code.google.com/p/pharo/issues/detail?id=4694
>>>
>>> 4695: Time asString prints nanos unrounded
>>> http://code.google.com/p/pharo/issues/detail?id=4695
>>>
>>>
>>> --
>>> Andrea Brühlmann
>>>
>
>
> --
> AB    |   ANDREA BRÜHLMANN · SOFTWARE ENGINEER
>      |   NETSTYLE · TERRASSENWEG 18 · CH-3012 BERN
>      |   TEL  +41 31 356 42 54 · FAX  +41 31 356 42 51
>      |   WWW.NETSTYLE.CH · [hidden email]
>


Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Nicolas Cellier
2011/9/5 Stéphane Ducasse <[hidden email]>:
> BTW mike roberts is building a tool to be able to understand and fix the code range of the debugger hilighting.
> Probably your bug is also related to the closure introduction.
>
> Now the abstraction used is so low level that this is a pain and fragile.
> Ideally using an AST would be much better but this is for the future.
>
> Stef
>

Stef,
If you are saying that implementation is not crystal clear, I can only
agree with you.
Eliot did avoid a full rewrite (I guess he took the shortest path to
make the available implementation work with closures), and this design
decision can be questioned indeed.

But speaking of abstraction level by itself, I don't understand your sentence.
To me, the abstraction is at the correct level.
The Debugger has to map the low level execution machinery
(Context/program counter/CompiledMethod/byteCodes) to high level
specification visible to the user (source code).
The Debugger does so by mapping bytecodes to source ranges via AST, no
more, no less.

In case of inlined blocks, there are more instructions on the bytecode
side than messages sent on the AST side, just to make the problem a
bit more complex, so maybe there are more intelligent way to address
the mapping problem (maybe you mean via an intermediate transformation
of AST), but you'll have to explain this a bit deeper.

Nicolas

>
> On Sep 5, 2011, at 9:07 AM, Andrea Brühlmann wrote:
>
>> Hello Stef,
>>
>> I appreciate the work of the pharo community! We will continue reporting bugs and submit fixes that
>> we made. Of course I do not expect anyone to fix all reported bugs, but there are issues like 4694
>> (debugger) where we really need your help. Other issues like the buggy code completion can be
>> workarounded by me by disabling code completion ;-)
>>
>> So thanks for the welcome and I am looking forward to having 4694 fixed!
>>
>> Andrea
>>
>>
>>
>>
>> Stéphane Ducasse schrieb:
>>> Thanks andrea
>>> Welcome to the pharo mailing-list and community. Now I think that this is important that inside netstyle you also consider that if pharo is important for you (which I imagine) that you should also contribute. Bug reporting is already a contribution.  Pharo is an open-source software mainly supported by the free time of people, people that are often do not earning their money from their pharo work (I thank them for all that). I also understand that people can get frustrated by changes but what can we do?
>>> May be people can gather and check the fixes that are important to fix but we do not have the force to maintain.
>>> Imagine well that we have one engineer full time since 8 months. Stef
>>> On Aug 23, 2011, at 8:54 AM, Andrea Brühlmann wrote:
>>>> Hello,
>>>>
>>>> I reported some bugs on the issue tracker and hope you can help me with them!
>>>>
>>>>
>>>> 4681: Accepting with the enter key does not work
>>>>     http://code.google.com/p/pharo/issues/detail?id=4681
>>>>
>>>> 4682: Code completion makes strange cursor placements and too many spaces
>>>>     http://code.google.com/p/pharo/issues/detail?id=4682
>>>>
>>>> 4683: Code completion breaks some search fields (errors during typing)
>>>>     http://code.google.com/p/pharo/issues/detail?id=4683
>>>>
>>>> 4684: Missing ctrl+w (methodNamesContainingIt:)
>>>>     http://code.google.com/p/pharo/issues/detail?id=4684
>>>>
>>>> 4685: Merge dialog: cannot resolve conflict with removed method
>>>>     http://code.google.com/p/pharo/issues/detail?id=4685
>>>>
>>>> 4686: Method category change creates no new version
>>>>     http://code.google.com/p/pharo/issues/detail?id=4686
>>>>
>>>> 4687: Errors during coding (MessageNotUnderstood: receiver of "morph" is nil)
>>>>     http://code.google.com/p/pharo/issues/detail?id=4687
>>>>
>>>> 4688: Progress bar disappears on image save
>>>>     http://code.google.com/p/pharo/issues/detail?id=4688
>>>>
>>>> 4689: MessageTally bug
>>>>     http://code.google.com/p/pharo/issues/detail?id=4689
>>>>
>>>> 4690: Progress bar position
>>>>     http://code.google.com/p/pharo/issues/detail?id=4690
>>>>
>>>> 4691: Bad line breaks in code until window is resized (because of bold text?)
>>>>     http://code.google.com/p/pharo/issues/detail?id=4691
>>>>
>>>> 4692: Drop downs should select the default entry (topmost) instead of a middle one
>>>>     http://code.google.com/p/pharo/issues/detail?id=4692
>>>>
>>>> 4693: Line breaks in tooltips are wrong
>>>>     http://code.google.com/p/pharo/issues/detail?id=4693
>>>>
>>>> 4694: Debugger: stepping over an error can not open a new debugger until I hit cmd+.
>>>>     http://code.google.com/p/pharo/issues/detail?id=4694
>>>>
>>>> 4695: Time asString prints nanos unrounded
>>>>     http://code.google.com/p/pharo/issues/detail?id=4695
>>>>
>>>>
>>>> --
>>>> Andrea Brühlmann
>>>>
>>
>>
>> --
>> AB    |   ANDREA BRÜHLMANN · SOFTWARE ENGINEER
>>      |   NETSTYLE · TERRASSENWEG 18 · CH-3012 BERN
>>      |   TEL  +41 31 356 42 54 · FAX  +41 31 356 42 51
>>      |   WWW.NETSTYLE.CH · [hidden email]
>>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Nicolas Cellier
2011/9/5 Nicolas Cellier <[hidden email]>:

> 2011/9/5 Stéphane Ducasse <[hidden email]>:
>> BTW mike roberts is building a tool to be able to understand and fix the code range of the debugger hilighting.
>> Probably your bug is also related to the closure introduction.
>>
>> Now the abstraction used is so low level that this is a pain and fragile.
>> Ideally using an AST would be much better but this is for the future.
>>
>> Stef
>>
>
> Stef,
> If you are saying that implementation is not crystal clear, I can only
> agree with you.
> Eliot did avoid a full rewrite (I guess he took the shortest path to
> make the available implementation work with closures), and this design
> decision can be questioned indeed.
>
> But speaking of abstraction level by itself, I don't understand your sentence.
> To me, the abstraction is at the correct level.
> The Debugger has to map the low level execution machinery
> (Context/program counter/CompiledMethod/byteCodes) to high level
> specification visible to the user (source code).
> The Debugger does so by mapping bytecodes to source ranges via AST, no
> more, no less.
>
> In case of inlined blocks, there are more instructions on the bytecode
> side than messages sent on the AST side, just to make the problem a
> bit more complex, so maybe there are more intelligent way to address
> the mapping problem (maybe you mean via an intermediate transformation
> of AST), but you'll have to explain this a bit deeper.
>
> Nicolas
>

By the way, I would find it cool to have an optional byteCode view
parallel to source code view and see the execution of byte codes, and
why not, a view of Context stack frames.
Also, the debugger might step message by message (AST-based) rather
than byteCode by byteCode, is this what you mean by wrong abstraction
level ?

Nicolas

>>
>> On Sep 5, 2011, at 9:07 AM, Andrea Brühlmann wrote:
>>
>>> Hello Stef,
>>>
>>> I appreciate the work of the pharo community! We will continue reporting bugs and submit fixes that
>>> we made. Of course I do not expect anyone to fix all reported bugs, but there are issues like 4694
>>> (debugger) where we really need your help. Other issues like the buggy code completion can be
>>> workarounded by me by disabling code completion ;-)
>>>
>>> So thanks for the welcome and I am looking forward to having 4694 fixed!
>>>
>>> Andrea
>>>
>>>
>>>
>>>
>>> Stéphane Ducasse schrieb:
>>>> Thanks andrea
>>>> Welcome to the pharo mailing-list and community. Now I think that this is important that inside netstyle you also consider that if pharo is important for you (which I imagine) that you should also contribute. Bug reporting is already a contribution.  Pharo is an open-source software mainly supported by the free time of people, people that are often do not earning their money from their pharo work (I thank them for all that). I also understand that people can get frustrated by changes but what can we do?
>>>> May be people can gather and check the fixes that are important to fix but we do not have the force to maintain.
>>>> Imagine well that we have one engineer full time since 8 months. Stef
>>>> On Aug 23, 2011, at 8:54 AM, Andrea Brühlmann wrote:
>>>>> Hello,
>>>>>
>>>>> I reported some bugs on the issue tracker and hope you can help me with them!
>>>>>
>>>>>
>>>>> 4681: Accepting with the enter key does not work
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4681
>>>>>
>>>>> 4682: Code completion makes strange cursor placements and too many spaces
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4682
>>>>>
>>>>> 4683: Code completion breaks some search fields (errors during typing)
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4683
>>>>>
>>>>> 4684: Missing ctrl+w (methodNamesContainingIt:)
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4684
>>>>>
>>>>> 4685: Merge dialog: cannot resolve conflict with removed method
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4685
>>>>>
>>>>> 4686: Method category change creates no new version
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4686
>>>>>
>>>>> 4687: Errors during coding (MessageNotUnderstood: receiver of "morph" is nil)
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4687
>>>>>
>>>>> 4688: Progress bar disappears on image save
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4688
>>>>>
>>>>> 4689: MessageTally bug
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4689
>>>>>
>>>>> 4690: Progress bar position
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4690
>>>>>
>>>>> 4691: Bad line breaks in code until window is resized (because of bold text?)
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4691
>>>>>
>>>>> 4692: Drop downs should select the default entry (topmost) instead of a middle one
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4692
>>>>>
>>>>> 4693: Line breaks in tooltips are wrong
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4693
>>>>>
>>>>> 4694: Debugger: stepping over an error can not open a new debugger until I hit cmd+.
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4694
>>>>>
>>>>> 4695: Time asString prints nanos unrounded
>>>>>     http://code.google.com/p/pharo/issues/detail?id=4695
>>>>>
>>>>>
>>>>> --
>>>>> Andrea Brühlmann
>>>>>
>>>
>>>
>>> --
>>> AB    |   ANDREA BRÜHLMANN · SOFTWARE ENGINEER
>>>      |   NETSTYLE · TERRASSENWEG 18 · CH-3012 BERN
>>>      |   TEL  +41 31 356 42 54 · FAX  +41 31 356 42 51
>>>      |   WWW.NETSTYLE.CH · [hidden email]
>>>
>>
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Marcus Denker-4
In reply to this post by Nicolas Cellier

On Sep 5, 2011, at 2:06 PM, Nicolas Cellier wrote:
>
> By the way, I would find it cool to have an optional byteCode view
> parallel to source code view and see the execution of byte codes, and
> why not, a view of Context stack frames.
> Also, the debugger might step message by message (AST-based) rather
> than byteCode by byteCode, is this what you mean by wrong abstraction
> level ?
>
With the wrong abstraction level: Why do we care about stepping bytecodes
at all? Why not only have an AST interpreter based debugger?

        Marcus



--
Marcus Denker -- http://marcusdenker.de


Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Nicolas Cellier
2011/9/5 Marcus Denker <[hidden email]>:

>
> On Sep 5, 2011, at 2:06 PM, Nicolas Cellier wrote:
>>
>> By the way, I would find it cool to have an optional byteCode view
>> parallel to source code view and see the execution of byte codes, and
>> why not, a view of Context stack frames.
>> Also, the debugger might step message by message (AST-based) rather
>> than byteCode by byteCode, is this what you mean by wrong abstraction
>> level ?
>>
> With the wrong abstraction level: Why do we care about stepping bytecodes
> at all? Why not only have an AST interpreter based debugger?
>
>        Marcus
>
>

OK, then I finally succeeded in decoding Marcus/Stef high level
instructions into low level specifications.
I translate here for those obscurantists speaking native gdb: you want
step/jump instead of stepi/jumpi.
But it would have been better to provide detailed explanations from
the beginning for slow brained like me ;).

Nonetheless, you still need to map execution machiney to source code
because of initial program counter, and in case of AST-based stepping,
the map must be bi-directional in order to execute multiple bytecodes
sometimes required by a single instruction.

Nicolas

>
> --
> Marcus Denker -- http://marcusdenker.de
>
>
>

123