Thanks Thomas :) I'll take a look!
On Wed, Nov 15, 2017 at 6:29 PM, Thomas Dupriez < [email protected]> wrote: > Here they are: > > https://github.com/dupriezt/Pharo-RunToSelectionDebuggerFeature > > Le 15/11/2017 à 10:47, Guillermo Polito a écrit : > > Thomas, can you try writing some real test cases in SUnit? > > I don't care if they are originally red (they will be because it is not > working). > > The idea is that we capture all these different scenarios and have a way > to know automatically in the future if > - we fixed all them > - or we have a regression and broke some. > > On Wed, Nov 15, 2017 at 10:44 AM, Thomas Dupriez < > [email protected]> wrote: > >> >> >> Le 15/11/2017 à 01:19, Ben Coman a écrit : >> >> >> >> On 14 November 2017 at 23:55, Thomas Dupriez < >> [email protected]> wrote: >> >>> While working on writing a test, I discovered that just using >>> stepThrough instead of stepOver may not be the definitive solution. >>> >>> # Example1: When using the `value` message directly (instead of calling >>> a method that does that as in my previous example), the RunToHere moves the >>> debugSession to the `value` message and not the inside of the block. >>> >>> `meth2 >>> self halt. >>> [ >>> 1+1. >>> 2+2."< cursor before the RunToHere" >>> ] ">"value"<result of the RunToHere". >>> 3+3. >>> "Two stepInto are required after the RunToHere to reach the cursor >>> (the `+2` message)"` >>> >>> # Example2: Going back to using a dedicated method for evaluating the >>> block, but increasing the block's size. The RunToHere moves the >>> debugSession to the first message of the block instead of the pointed one. >>> >>> `meth3 >>> self halt. >>> MyClass new evalBlock: >>> [ >>> 1">"+1"< result of the RunToHere". >>> 2+2."< cursor before the RunToHere" >>> ]. >>> 3+3. >>> "One stepInto is required after the RunToHere to reach the cursor >>> (the `+2` message)"` >>> >>> cheers -thomas >>> >> >> In both cases, what happens with an additional step-Through? >> >> cheers -ben >> >> >> # Example1*: >> Operation: RunToHere with cursor at the end of the `2+2.` line + 1 >> stepThrough. >> Result: DebugSession at the `+1`, first instruction of the block but not >> the intended position. >> Note: Doing an additional stepThrough brings the debugSession to the >> intended `+2`. >> >> `meth2 >> self halt. >> [ >> 1">"+1"<"."result of the RunToHere + 1 stepThrough" >> 2+2."< cursor before the RunToHere" >> ] ">"value"<result of the RunToHere". >> 3+3. >> "Two stepInto are required after the RunToHere to reach the cursor >> (the `+2` message)"` >> >> # Example2*: >> Operation: RunToHere with cursor at the end of the `2+2.` line + 1 >> stepThrough >> Result: DebugSession at the intended `+2` position. >> Note: The additional stepThrough just moves the DebugSession to the next >> instruction. See Example3 for what happens when the target instruction is >> further away. >> >> `meth3 >> self halt. >> MyClass new evalBlock: >> [ >> 1">"+1"< result of the RunToHere". >> 2">"+2"<result of RunToHere+1stepThrough"."< cursor before the >> RunToHere" >> ]. >> 3+3. >> "One stepInto is required after the RunToHere to reach the cursor >> (the `+2` message)"` >> >> >> # Example3: >> Operation: RunToHere with cursor at the end of the `3+3.` line. + 1 >> stepThrough. >> Result: DebugSession is at the `+2` instruction, not the intended `+3` >> one. >> >> `meth4 >> self halt. >> MyClass new evalBlock: >> [ >> 1">"+1"< result of the RunToHere". >> 2">"+2"<result of RunToHere+1stepThrough". >> 3">"+3"<result of RunToHere+2stepThrough"."< cursor before the >> RunToHere" >> ]. >> 4+4. >> "One stepInto is required after the RunToHere to reach the cursor >> (the `+2` message)"` >> >> cheers -thomas >> >> Le 13/11/2017 à 17:23, Thomas Dupriez a écrit : >>> >>> >>> >>> Le 13/11/2017 à 15:56, Ben Coman a écrit : >>> >>> >>> >>> On Mon, Nov 13, 2017 at 9:32 PM, Thomas Dupriez < >>> [email protected]> wrote: >>> >>>> >>>> >>>> Le 13/11/2017 à 14:08, Ben Coman a écrit : >>>> >>>> >>>> >>>> On Mon, Nov 13, 2017 at 8:40 PM, Thomas Dupriez < >>>> [email protected]> wrote: >>>> >>>>> I dug a bit in this issue. Here are the results: >>>>> >>>>> >>>>> # Problem raised by Stephanne >>>>> Code like the following is open in the debugger: >>>>> >>>>> `myMethod >>>>> 1+1. <PROGRAMCOUNTER> >>>>> MyClass new myEvalBlock: [ >>>>> 2+2. <CURSOR> >>>>> ]. >>>>> 3+3.` >>>>> >>>>> The program counter is on the 1+1, the cursor is at the end of the 2+2 >>>>> line. >>>>> Right-click, "Run to here". >>>>> -> the program counter moves to the 3+3, and does not stop at the 2+2 >>>>> (where the cursor is). Even though the 2+2 does get evaluated (the >>>>> myEvalBlock method evaluates the blocks it receives). >>>>> If there was no code after the block, the debugger would jump back to >>>>> the caller of myMethod. >>>>> >>>>> # Why this happens >>>>> The implementation of RunToHere (source code below) is basically to >>>>> stepOver until the context is different or the source code position of the >>>>> program counter is higher or equal to the source code position of the >>>>> cursor/selection. >>>>> >>>>> Since the debugger uses stepOver, it executes myEvalBlock without >>>>> stopping, reaches the 3+3 and see it has gone further than the source code >>>>> position of the cursorm so it stops. >>>>> >>>>> Source code of DebugSession>>#runToSelection:inContext: >>>>> >>>>> `runToSelection: selectionInterval inContext: aContext >>>>> "Attempt to step over instructions in selectedContext until the >>>>> execution reaches the selected instruction. This happens when >>>>> the >>>>> program counter passes the begining of selectionInterval. >>>>> >>>>> A not nill and valid interval is expected." >>>>> >>>>> (self pcRangeForContext: aContext) first >= selectionInterval >>>>> first >>>>> ifTrue: [ ^self ]. >>>>> self stepOver: aContext. >>>>> [ aContext == self interruptedContext and: [ (self >>>>> pcRangeForContext: aContext) first < selectionInterval first ] ] >>>>> whileTrue: [ self stepOver: aContext ]` >>>>> >>>>> # Observations and thoughts >>>>> - Replacing the stepOver with a stepInto -> This made the RunToHere >>>>> stops when resolving the 'new' message (because the context changes). >>>>> >>>> >>>> Thanks for looking into this Thomas. >>>> What happens if you use stepThrough rather than stepInto? >>>> >>>> cheers -ben >>>> >>>> >>>> Using `stepThrough: aContext` instead of `stepInto: aContext`, the >>>> RunToHere in the block stops at the intended place: the 2+2. >>>> Printing the sequence of `self pcRangeForContext: aContext` yields the >>>> following. It doesn't show precisely the stop in the block, and instead >>>> shows an interval encompassing the whole block. That's what I was using to >>>> see where the execution was going during the RunToHere loop so I guess >>>> that's not a precise enough indicator for this situation... >>>> (21 to: 22)(34 to: 36)(49 to: 60)(38 to: 60)(38 to: 60) >>>> >>>> I also tried the case where the block does not get evaluated (changing >>>> myEvalBlock so that it does nothing). In this situation, the RunToHere with >>>> a stepThrough ends up at the intended place, the 3+3. >>>> >>>> So... just use stepThrough for the RunToHere I guess? >>>> >>> >>> Seems like expected behaviour. >>> Could you create an Issue/Pull Request to provide some concrete code to >>> review? >>> >>> Now the trick will be if you can devise some tests to go with this. >>> For examples perhaps look at senders of newDebugSessionNamed:startedAt: >>> including SpecDebugger>>testBasic. >>> >>> cheers -ben >>> >>> >>> >>> I opened an issue on FogBugz: https://pharo.fogbugz.com/f/cases/20687/ >>> >>> Attached are the file out of the test code (MyClass.st) and the fixed >>> runToSelection:inContext: (DebugSession-runToSelectioninContext). >>> I'll make a pull request tomorrow. >>> >>> For the test, I'm thinking about something like the following, that >>> checks the source code position of the pc after the runToSelection. (It's >>> not working yet) >>> Now that I think of it, the source code position of the pc will maybe be >>> the whole block (as experienced earlier) so this approach may end up not >>> working. >>> I'll come back to that tomorrow. >>> >>> Draft for a test: >>> `testRunToSelectionInContext >>> |context_ process_ session_| >>> "The variable names have an underscore to distinguish them from >>> those declared in the setUp method of this TestCase (DebuggerModelTest), >>> which are not suited for this test (because not making a debug session out >>> of suitable code)" >>> context_ := [1+1. >>> self evalBlock:[2+2]. >>> 3+3. >>> ] asContext. >>> process_ := Process >>> forContext: context_ >>> priority: Processor userInterruptPriority. >>> >>> session_:= process_ newDebugSessionNamed: 'test session' startedAt: >>> context_. >>> session_ runToSelection: (Interval from: 28 to: 27) inContext: >>> context_. >>> self assert: ((session_ pcRangeForContext: context_) first == 3)` >>> >>> cheers - Thomas >>> >>> >>>> Thomas >>>> >>>> >>>> >>>> >>>> >>>>> - Replacing the stepOver with stepInto and removing the equal >>>>> condition on contexts -> The RunToHere goes to the 3+3. Looking at the >>>>> source code position of the program counter, it doesn't enter the block >>>>> and >>>>> seems to resolve it in a single step. I don't really get why that is, >>>>> considering using the stepInto button of the debugger does enter the >>>>> block. >>>>> Here is the series of source code positions of the program counter during >>>>> the RunToHere: (21 to: 22)(34 to: 36)(34 to: 36)(49 to: 60)(38 to: 60)(38 >>>>> to: 60)(65 to: 66). >>>>> However, removing the equal condition on contexts means that if the >>>>> method call returns before reaching the cursor, it won't stop! >>>>> >>>>> - An idea could be to have the RunToHere place a metalink on the >>>>> selected node and let the execution run until it hits the metalink, which >>>>> then updates the debugger. Potential problems are that it implies >>>>> installing a metalink on a method that is already on the stack, which may >>>>> not be that easy to do properly (in particular, it affects the program >>>>> counter since it changes the bytecode), and there is the potential case >>>>> where the metalink is never reached (for example imagine the myEvalBlock: >>>>> method of my example is just storing the block and not evaluating it). >>>>> >>>>> >>>>> Cheers, >>>>> Thomas >>>>> >>>>> >>>>> >>>>> Le 09/11/2017 à 22:06, Stephane Ducasse a écrit : >>>>> >>>>>> Agreed. Thomas? It would be a good bone to ... (Un bon os a ronger) . >>>>>> >>>>>> Stef >>>>>> >>>>>> On Thu, Nov 9, 2017 at 4:32 AM, Tudor Girba <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> The basic tools, such as debugger, are expected to work. If >>>>>>> something does not work, it’s a bug. >>>>>>> >>>>>>> Cheers, >>>>>>> Doru >>>>>>> >>>>>>> >>>>>>> On Nov 8, 2017, at 11:59 PM, Tim Mackinnon <[email protected]> >>>>>>>> <[email protected]> wrote: >>>>>>>> >>>>>>>> I think it's broken in Pharo 6 too, as I often find it unreliable. >>>>>>>> >>>>>>>> It's hard to know what should work anymore - we really need a >>>>>>>> stabilisation release to let the dust settle. >>>>>>>> >>>>>>>> I'm always a bit reticent to report things as I'm not sure what you >>>>>>>> expect to work. >>>>>>>> >>>>>>>> Tim >>>>>>>> >>>>>>>> Sent from my iPhone >>>>>>>> >>>>>>>> On 8 Nov 2017, at 20:40, Stephane Ducasse <[email protected]> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> Hi >>>>>>>>> >>>>>>>>> I have the following method and I have my cursor -MY CURSOR HERE- >>>>>>>>> I select the menu run to here and .... I exit the method. >>>>>>>>> :( >>>>>>>>> >>>>>>>>> Is run to here working in Pharo 70? >>>>>>>>> I start to get worry about the number of bugs I get when using >>>>>>>>> Pharo70. >>>>>>>>> >>>>>>>>> Stef >>>>>>>>> >>>>>>>>> >>>>>>>>> fileOut >>>>>>>>> "File out the receiver, to a file whose name is a function of the >>>>>>>>> change-set name and a unique numeric tag." >>>>>>>>> >>>>>>>>> | nameToUse | >>>>>>>>> self halt. >>>>>>>>> self class promptForDefaultChangeSetDirectoryIfNecessary. >>>>>>>>> nameToUse := (self defaultChangeSetDirectory / self name , 'cs') >>>>>>>>> nextVersion basename. >>>>>>>>> UIManager default >>>>>>>>> showWaitCursorWhile: >>>>>>>>> [ >>>>>>>>> | internalStream | >>>>>>>>> internalStream := (String new: 10000) writeStream. >>>>>>>>> >>>>>>>>> -MY CURSOR HERE- >>>>>>>>> >>>>>>>>> internalStream >>>>>>>>> header; >>>>>>>>> timeStamp. >>>>>>>>> self fileOutPreambleOn: internalStream. >>>>>>>>> self fileOutOn: internalStream. >>>>>>>>> self fileOutPostscriptOn: internalStream. >>>>>>>>> CodeExporter >>>>>>>>> writeSourceCodeFrom: internalStream >>>>>>>>> baseName: (nameToUse copyFrom: 1 to: nameToUse size - 3) >>>>>>>>> isSt: false ] >>>>>>>>> >>>>>>>>> >>>>>>>> -- >>>>>>> www.tudorgirba.com >>>>>>> www.feenk.com >>>>>>> >>>>>>> "Value is always contextual." >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>> >>>>> >>>> >>>> >>> >>> >>> >> >> > > > -- > > > > Guille Polito > > Research Engineer > > Centre de Recherche en Informatique, Signal et Automatique de Lille > > CRIStAL - UMR 9189 > > French National Center for Scientific Research - *http://www.cnrs.fr > <http://www.cnrs.fr>* > > > *Web:* *http://guillep.github.io* <http://guillep.github.io> > > *Phone: *+33 06 52 70 66 13 <+33%206%2052%2070%2066%2013> > > > -- Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - *http://www.cnrs.fr <http://www.cnrs.fr>* *Web:* *http://guillep.github.io* <http://guillep.github.io> *Phone: *+33 06 52 70 66 13
