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 <thomas.dupriez@ens-paris- > saclay.fr> 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
