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

Stéphane Ducasse
We were thinking with marcus that using byte code to go back to sources code is
broken by essence and that using an AST is the way to go.
Now we would need a good AST-based interpreter to start with.

> 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

Stéphane Ducasse
In reply to this post by Nicolas Cellier
>>
>
> 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 ?


yes :)


Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Stéphane Ducasse
In reply to this post by Nicolas Cellier
>
> 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


yes :)
we were not clear. Do not worry.

Stef

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Lukas Renggli
In reply to this post by Stéphane Ducasse
Even if you have a AST debugger you still need at some point a bytecode to AST (or source) mapping, because otherwise you cannot just break into a debugger at a random point in the code. I imagine this jump from bytecode to AST interpretation quite difficult; likely you still need a full bytecode stepper to find a position where you can jump into the AST interpreter.

Lukas

On Monday, 5 September 2011, Stéphane Ducasse <[hidden email]> wrote:
> We were thinking with marcus that using byte code to go back to sources code is
> broken by essence and that using an AST is the way to go.
> Now we would need a good AST-based interpreter to start with.
>
>> 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
>>>>>

--
Lukas Renggli
www.lukas-renggli.ch
Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Eliot Miranda-2


On Mon, Sep 5, 2011 at 8:02 AM, Lukas Renggli <[hidden email]> wrote:
Even if you have a AST debugger you still need at some point a bytecode to AST (or source) mapping, because otherwise you cannot just break into a debugger at a random point in the code. I imagine this jump from bytecode to AST interpretation quite difficult; likely you still need a full bytecode stepper to find a position where you can jump into the AST interpreter.

Right.  Basically the two levels, source and bytecode are all one needs and a fixation with ASs doesn't really help.  The bytecode is quite tractible, and look at the different performance/quality of Smalltalk vs Ruby for what bytecode vs AST architectures get you.


Lukas


On Monday, 5 September 2011, Stéphane Ducasse <[hidden email]> wrote:
> We were thinking with marcus that using byte code to go back to sources code is
> broken by essence and that using an AST is the way to go.
> Now we would need a good AST-based interpreter to start with.
>
>> 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
>>>>>

--
Lukas Renggli
www.lukas-renggli.ch



--
best,
Eliot

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Eliot Miranda-2
In reply to this post by Nicolas Cellier
Hi Nicolas,

    that's quite a lot to absorb.  Exactly what do you want me to review?  The first thing it seems to me is to define what we want to be highlighted at different bytecodes in an optimized loop.  A second thing is to think about how the debugger can avoid stepping through the hidden sends that are part of a loop.  What I mean is that 

    1 to: n do: [:i| ....]

is actually
     i := 1.
     [i <= n] whileTrue: [

and so the debugger steps through i:= 1 and i <= n, even though these aren't visible in the source.  That's why one has to click several times to get into the body of the loop.  The debugger could for example use the source ranges for the bytecodes to step until the source range changes.  So if there is the same source range info for all the bytecodes involved in the loop iteration (not the loop body) the debugger will be able to respond to step properly.  What do you think?

[sorry for not having responded sooner; esug and personal issues have got in the way. apologies]

On Thu, Aug 25, 2011 at 1:06 PM, Nicolas Cellier <[hidden email]> wrote:
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
>>
>
>
>




--
best,
Eliot

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Michael van der Gulik-2
In reply to this post by Marcus Denker-4
On Tue, Sep 6, 2011 at 12:10 AM, Marcus Denker <[hidden email]> wrote:

>
> 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?

Because the debugger might be looking at bytecodes that were generated
by something other than a Smalltalk compiler.

Michael.


--
http://gulik.pbwiki.com/

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Igor Stasenko
On 6 September 2011 07:41, Michael van der Gulik <[hidden email]> wrote:

> On Tue, Sep 6, 2011 at 12:10 AM, Marcus Denker <[hidden email]> wrote:
>>
>> 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?
>
> Because the debugger might be looking at bytecodes that were generated
> by something other than a Smalltalk compiler.
>

Then AST will serve for that even better, because a single AST node
could be represented by several bytecodes,
and by using a proper AST,  debugger will know for sure how to step
over single semantic element, written in your language.

> Michael.
>
>
> --
> http://gulik.pbwiki.com/
>
>



--
Best regards,
Igor Stasenko.

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Igor Stasenko
In reply to this post by Lukas Renggli
On 5 September 2011 18:02, Lukas Renggli <[hidden email]> wrote:
> Even if you have a AST debugger you still need at some point a bytecode to
> AST (or source) mapping, because otherwise you cannot just break into a
> debugger at a random point in the code. I imagine this jump from bytecode to
> AST interpretation quite difficult; likely you still need a full bytecode
> stepper to find a position where you can jump into the AST interpreter.
>

It depends, whether you able (or want to) insert a breakpoint which
are inside a single AST node (for AST node which contains multiple
bytecode),
or allow to halt only at AST nodes boundaries.

Consider C debugger - it has to deal with same situation: a single C
statement could contain multiple machine code instructions, but when
you stepping
over that statement, you are not stepping over single machine code
instruction(s) at a time, you stepping over single C statement.

In some cases, stepping over single machine instruction is useful
(especially when you dealing with generated machine code ;)
but when you debugging a normal code, you don't need that, which means
that if you have an AST -> bytecode ranges mapping
you know the ranges and stepping points and don't have to interpret
each bytecode separately, but intead just interpret what AST node
does.

> Lukas
>
> On Monday, 5 September 2011, Stéphane Ducasse <[hidden email]>
> wrote:
>> We were thinking with marcus that using byte code to go back to sources
>> code is
>> broken by essence and that using an AST is the way to go.
>> Now we would need a good AST-based interpreter to start with.
>>
>>> 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
>>>>>>
>
> --
> Lukas Renggli
> www.lukas-renggli.ch
>



--
Best regards,
Igor Stasenko.

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Michael Roberts-2
In reply to this post by Eliot Miranda-2
Hi Eliot,

Nicolas' change that we put on the tracker didn't really work as
intended (Nicolas did you see that?), but i'll let him respond to the
points he was making.

For me (point #1) It would be better for you to try and tell us the
classes/methods that are either obviously broken or need the attention
to get to a series of fixes.

e.g., ignoring the loop highlight for the moment, the pc mapping goes
wrong right at the start of this example:

toDoOutsideTemp
        | temp collection |
        collection := OrderedCollection new.
        1 to: 5 do: [ :index |
                temp := index.
                collection add: [ temp ] ].
       
by highlighting pc=2 (IIRC) which is the assignment, but the first
highlight should be pc=3 which is the send of #new. It doesn't
correctly map/avoid the extra instructions for the array
initialisation for copying the values outside the block. is it
possible to fix that first? where do we look?

I am just wondering how this can be approached in stages if possible.
i know it is all related at some level.  stepping into the code/method
that does the majority of the pc mapping is a funny experience ;-)
because the method is structured in such a way to expose a number of
these bugs.

for point #2 ideally we would not ask for a step and have nothing
happen. However i wonder if we can get the highlight areas correct,
even if we have to "step" through hidden parts. This assumes we can
suppress the highlight for the hidden bits which i think we already
have...but only if we can do one without the other.  I would then
write tests for the highlights, and we would have some protection for
further changes.

thanks,
Mike


On Mon, Sep 5, 2011 at 5:14 PM, Eliot Miranda <[hidden email]> wrote:

> Hi Nicolas,
>     that's quite a lot to absorb.  Exactly what do you want me to review?
>  The first thing it seems to me is to define what we want to be highlighted
> at different bytecodes in an optimized loop.  A second thing is to think
> about how the debugger can avoid stepping through the hidden sends that are
> part of a loop.  What I mean is that
>     1 to: n do: [:i| ....]
> is actually
>      i := 1.
>      [i <= n] whileTrue: [
> and so the debugger steps through i:= 1 and i <= n, even though these aren't
> visible in the source.  That's why one has to click several times to get
> into the body of the loop.  The debugger could for example use the source
> ranges for the bytecodes to step until the source range changes.  So if
> there is the same source range info for all the bytecodes involved in the
> loop iteration (not the loop body) the debugger will be able to respond to
> step properly.  What do you think?
>
> [sorry for not having responded sooner; esug and personal issues have got in
> the way. apologies]
> On Thu, Aug 25, 2011 at 1:06 PM, Nicolas Cellier
> <[hidden email]> wrote:
>>
>> 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
>> >>
>> >
>> >
>> >
>>
>
>
>
> --
> best,
> Eliot
>

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Eliot Miranda-2


On Tue, Sep 6, 2011 at 2:10 PM, Michael Roberts <[hidden email]> wrote:
Hi Eliot,

Nicolas' change that we put on the tracker didn't really work as
intended (Nicolas did you see that?), but i'll let him respond to the
points he was making.

For me (point #1) It would be better for you to try and tell us the
classes/methods that are either obviously broken or need the attention
to get to a series of fixes.

e.g., ignoring the loop highlight for the moment, the pc mapping goes
wrong right at the start of this example:

toDoOutsideTemp
       | temp collection |
       collection := OrderedCollection new.
       1 to: 5 do: [ :index |
               temp := index.
               collection add: [ temp ] ].

by highlighting pc=2 (IIRC) which is the assignment, but the first
highlight should be pc=3 which is the send of #new. It doesn't
correctly map/avoid the extra instructions for the array
initialisation for copying the values outside the block. is it
possible to fix that first? where do we look?

Well, this works in Squeak.  The pc ranges must be determined after generating the method *with closure analysis*.  i.e. the code ranges must be derived form the same resulting bytecode.  To derive 2 for the pc of the send of new the system I guess must be compiling for source ranges differently from normal compilation.  Haver you compares Squeak to Pharo for this example?


I am just wondering how this can be approached in stages if possible.
i know it is all related at some level.  stepping into the code/method
that does the majority of the pc mapping is a funny experience ;-)
because the method is structured in such a way to expose a number of
these bugs.

for point #2 ideally we would not ask for a step and have nothing
happen. However i wonder if we can get the highlight areas correct,
even if we have to "step" through hidden parts. This assumes we can
suppress the highlight for the hidden bits which i think we already
have...but only if we can do one without the other.  I would then
write tests for the highlights, and we would have some protection for
further changes.

Right.  First thing is to get the source ranges correct.  Next thing is to modify the debugger so that step has a visible effect, i.e. that the debugger steps until the source range changes.
 

thanks,
Mike


On Mon, Sep 5, 2011 at 5:14 PM, Eliot Miranda <[hidden email]> wrote:
> Hi Nicolas,
>     that's quite a lot to absorb.  Exactly what do you want me to review?
>  The first thing it seems to me is to define what we want to be highlighted
> at different bytecodes in an optimized loop.  A second thing is to think
> about how the debugger can avoid stepping through the hidden sends that are
> part of a loop.  What I mean is that
>     1 to: n do: [:i| ....]
> is actually
>      i := 1.
>      [i <= n] whileTrue: [
> and so the debugger steps through i:= 1 and i <= n, even though these aren't
> visible in the source.  That's why one has to click several times to get
> into the body of the loop.  The debugger could for example use the source
> ranges for the bytecodes to step until the source range changes.  So if
> there is the same source range info for all the bytecodes involved in the
> loop iteration (not the loop body) the debugger will be able to respond to
> step properly.  What do you think?
>
> [sorry for not having responded sooner; esug and personal issues have got in
> the way. apologies]
> On Thu, Aug 25, 2011 at 1:06 PM, Nicolas Cellier
> <[hidden email]> wrote:
>>
>> 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
>> >>
>> >
>> >
>> >
>>
>
>
>
> --
> best,
> Eliot
>




--
best,
Eliot

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Eliot Miranda-2


On Tue, Sep 6, 2011 at 2:37 PM, Eliot Miranda <[hidden email]> wrote:


On Tue, Sep 6, 2011 at 2:10 PM, Michael Roberts <[hidden email]> wrote:
Hi Eliot,

Nicolas' change that we put on the tracker didn't really work as
intended (Nicolas did you see that?), but i'll let him respond to the
points he was making.

For me (point #1) It would be better for you to try and tell us the
classes/methods that are either obviously broken or need the attention
to get to a series of fixes.

e.g., ignoring the loop highlight for the moment, the pc mapping goes
wrong right at the start of this example:

toDoOutsideTemp
       | temp collection |
       collection := OrderedCollection new.
       1 to: 5 do: [ :index |
               temp := index.
               collection add: [ temp ] ].

by highlighting pc=2 (IIRC) which is the assignment, but the first
highlight should be pc=3 which is the send of #new. It doesn't
correctly map/avoid the extra instructions for the array
initialisation for copying the values outside the block. is it
possible to fix that first? where do we look?

Well, this works in Squeak.  The pc ranges must be determined after generating the method *with closure analysis*.  i.e. the code ranges must be derived form the same resulting bytecode.  To derive 2 for the pc of the send of new the system I guess must be compiling for source ranges differently from normal compilation.  Haver you compares Squeak to Pharo for this example?

and then things get better, but not ideal, if the send of startOfLastStatement: is removed from Parser>>statements:innerBlock:blockNode:, i.e. if

hereType ~~ #rightBracket ifTrue:
[[theBlockNode startOfLastStatement: (start := self startOfNextToken).
 (returns := self matchReturn)
...
becomes
hereType ~~ #rightBracket ifTrue:
[[start := self startOfNextToken.
 (returns := self matchReturn)
...

I think my confusion was that I wanted to remember in the block node the /end/ of the last statement, not the beginning, and I wasn't thinking at all clearly at the time.  Looks like startOfLastStatement in BlockNode should be removed.

With the above change the debugger highlights the "to: limit do:", but not the entire block as it would for a non-optimized block.  For me, I want the debugger to highlight the entire thing, from to: expr though to the closing ']'.  So I think more needs to be done (i.e. note the position of the closing ']' and pass this to the BlockNode).


I am just wondering how this can be approached in stages if possible.
i know it is all related at some level.  stepping into the code/method
that does the majority of the pc mapping is a funny experience ;-)
because the method is structured in such a way to expose a number of
these bugs.

for point #2 ideally we would not ask for a step and have nothing
happen. However i wonder if we can get the highlight areas correct,
even if we have to "step" through hidden parts. This assumes we can
suppress the highlight for the hidden bits which i think we already
have...but only if we can do one without the other.  I would then
write tests for the highlights, and we would have some protection for
further changes.

Right.  First thing is to get the source ranges correct.  Next thing is to modify the debugger so that step has a visible effect, i.e. that the debugger steps until the source range changes.
 

thanks,
Mike


On Mon, Sep 5, 2011 at 5:14 PM, Eliot Miranda <[hidden email]> wrote:
> Hi Nicolas,
>     that's quite a lot to absorb.  Exactly what do you want me to review?
>  The first thing it seems to me is to define what we want to be highlighted
> at different bytecodes in an optimized loop.  A second thing is to think
> about how the debugger can avoid stepping through the hidden sends that are
> part of a loop.  What I mean is that
>     1 to: n do: [:i| ....]
> is actually
>      i := 1.
>      [i <= n] whileTrue: [
> and so the debugger steps through i:= 1 and i <= n, even though these aren't
> visible in the source.  That's why one has to click several times to get
> into the body of the loop.  The debugger could for example use the source
> ranges for the bytecodes to step until the source range changes.  So if
> there is the same source range info for all the bytecodes involved in the
> loop iteration (not the loop body) the debugger will be able to respond to
> step properly.  What do you think?
>
> [sorry for not having responded sooner; esug and personal issues have got in
> the way. apologies]
> On Thu, Aug 25, 2011 at 1:06 PM, Nicolas Cellier
> <[hidden email]> wrote:
>>
>> 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
>> >>
>> >
>> >
>> >
>>
>
>
>
> --
> best,
> Eliot
>




--
best,
Eliot




--
best,
Eliot

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Michael van der Gulik-2
In reply to this post by Igor Stasenko
On Tue, Sep 6, 2011 at 8:12 PM, Igor Stasenko <[hidden email]> wrote:

> On 6 September 2011 07:41, Michael van der Gulik <[hidden email]> wrote:
>> On Tue, Sep 6, 2011 at 12:10 AM, Marcus Denker <[hidden email]> wrote:
>>>
>>> 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?
>>
>> Because the debugger might be looking at bytecodes that were generated
>> by something other than a Smalltalk compiler.
>>
>
> Then AST will serve for that even better, because a single AST node
> could be represented by several bytecodes,
> and by using a proper AST,  debugger will know for sure how to step
> over single semantic element, written in your language.

The bytecodes may not have come from an AST or language.

Examples:
* Aspect-oriented programming, e.g. automatic logging before and after
a method using bytecode injection.
* Automatically generated objects, e.g. proxies that filter and send
messages to another object.
* Hand-crafted bytecodes (for... er... fun)
* Compiler bugs - you want the debugger to actually be useful in
finding incorrectly generated methods.
* Commercial software, with no source code or obfuscated source code,
and possibly obfuscated bytecodes if that's somehow possible.

My point of view is that the debugger should be debugging bytecodes,
and stepping through the original source code is a nice extra feature.
This conversation is relevant to me because eventually I'll be making
changes to the debugger as well.

Michael.

--
http://gulik.pbwiki.com/

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Andrea Brühlmann-2
In reply to this post by Andrea Brühlmann-2
I checked some of my previously reported bugs in pharo1.3 and added our fixes - thanks for
integrating them :-)

Please have a look at the following issues:

New:
4920: Copying a trait fails
4919: Class copying issues

Question:
4688: Progress bar disappears on image save => Does "Milestone-1.4" mean you will not put the fix
into 1.3? This would be too sad...


Not to forget:
 > 4689: MessageTally bug
 >     http://code.google.com/p/pharo/issues/detail?id=4689
 >

> 4684: Missing ctrl+w (methodNamesContainingIt:)
>     http://code.google.com/p/pharo/issues/detail?id=4684
>
> 4686: Method category change creates no new version
>     http://code.google.com/p/pharo/issues/detail?id=4686
>
> 4692: Drop downs should select the default entry (topmost) instead of a
> middle one
>     http://code.google.com/p/pharo/issues/detail?id=4692
>

--
Andrea Brühlmann

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Mariano Martinez Peck


On Thu, Oct 20, 2011 at 1:45 PM, Andrea Brühlmann <[hidden email]> wrote:
I checked some of my previously reported bugs in pharo1.3 and added our fixes - thanks for integrating them :-)

Please have a look at the following issues:

New:
4920: Copying a trait fails
4919: Class copying issues

Question:
4688: Progress bar disappears on image save => Does "Milestone-1.4" mean you will not put the fix into 1.3? +

yes
 
This would be too sad...


No. Too sad is being 1 year and a half to release one single release. Don't expect perfection in each release, because otherwise there won't be any release.
If you really need this issue, then apply the patch to your own image. Only SERIOUS bugs can be integrated in already released versions. That a progress bar disappears when saving the image doesn't seem to be that serious.

Cheers

 

Not to forget:

> 4689: MessageTally bug
>     http://code.google.com/p/pharo/issues/detail?id=4689
>
4684: Missing ctrl+w (methodNamesContainingIt:)
   http://code.google.com/p/pharo/issues/detail?id=4684

4686: Method category change creates no new version
   http://code.google.com/p/pharo/issues/detail?id=4686

4692: Drop downs should select the default entry (topmost) instead of a middle one
   http://code.google.com/p/pharo/issues/detail?id=4692


--
Andrea Brühlmann




--
Mariano
http://marianopeck.wordpress.com

Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Stéphane Ducasse
In reply to this post by Andrea Brühlmann-2
Thanks andrea.
We will a post release update.

Stef

On Oct 20, 2011, at 1:45 PM, Andrea Brühlmann wrote:

> I checked some of my previously reported bugs in pharo1.3 and added our fixes - thanks for integrating them :-)
>
> Please have a look at the following issues:
>
> New:
> 4920: Copying a trait fails
> 4919: Class copying issues
>
> Question:
> 4688: Progress bar disappears on image save => Does "Milestone-1.4" mean you will not put the fix into 1.3? This would be too sad...
>
>
> Not to forget:
> > 4689: MessageTally bug
> >     http://code.google.com/p/pharo/issues/detail?id=4689
> >
>> 4684: Missing ctrl+w (methodNamesContainingIt:)
>>    http://code.google.com/p/pharo/issues/detail?id=4684
>> 4686: Method category change creates no new version
>>    http://code.google.com/p/pharo/issues/detail?id=4686
>> 4692: Drop downs should select the default entry (topmost) instead of a middle one
>>    http://code.google.com/p/pharo/issues/detail?id=4692
>
> --
> Andrea Brühlmann
>


Reply | Threaded
Open this post in threaded view
|

Re: Some bugs

Andrea Brühlmann-2
In reply to this post by Mariano Martinez Peck

>
>> This would be too sad...
>>
>>
> No. Too sad is being 1 year and a half to release one single release. Don't
> expect perfection in each release, because otherwise there won't be any
> release.
> If you really need this issue, then apply the patch to your own image. Only
> SERIOUS bugs can be integrated in already released versions. That a progress
> bar disappears when saving the image doesn't seem to be that serious.
>

You are completely right  :-)

We just started to minimize the amount of our personal patches and are getting used to putting them
on the issue tracker. But you are right, we will never get rid of personal patches because we need
them before they are integrated! And of course progress bars are not so serious :-)

Thanks again for all your help.

Andrea

123