Ticket #258 (closed enhancement: fixed)

Opened 4 years ago

Last modified 4 years ago

Further improve step debugging

Reported by: stephan Owned by:
Priority: major Milestone: OTDT_1.3.0_M4
Component: debug Version: 1.3.0M3
Keywords: Cc:

Description

While updating the user guide (cf. #241) a few discrepancies between described and actual behavior came to attention.

At a closer look I see the following potential for improvements

  1. a callin binding is presented as the source for two different stack frames -> should better visualize progress
  2. don't jump to the playedBy position without showing "lifting" in the label
  3. reduce display of "line: not availabel"
  4. reduce unnecessary stepping, where the debugger shows no progress

References: #67, #175, #235

Change History

Changed 4 years ago by stephan

The following conceptual changes are under way:

re 1.: Efforts had been invested to map statements in the chaining wrapper (base side) to the callin position in the role. Additionally the callin wrapper (team side) naturally maps to the same position. Therefore I will change the first mapping and visualize the chaining wrapper by jumping to the name of the base method while showing a label like "Dispatch callins for bm".

In case several teams adapt the same base method the new behavior should give an idea of the selection which team is to receive the control at which point in time.

re 2.: All content of the callin wrapper will either appear at the line of the callin binding or be marked as STEP_OVER. So F5 will enter these calls in sequence: (base guard), lifting, (regular guard), role method -- between calls the debugger will always return to the callin binding. The lift method itself has the playedBy position, thus entering the lift method will jump to the playedBy declaration as desired.

We might want to try the following: hitting F5 works as described above, F6 directly jumps into the role method and F7 takes you out of the callin binding. Alternatively, selecting the role method spec and using step into selection might be suitable (and doesn't break the semantics of F6 as step over).

re 3.: Specifically the initial wrapper (original base method name and signature) consists only of step-over and step-into. So there's actually no useful line information in that method. (Otherwise the debugger would unnecessarily stop while stepping into). Since this stack frame adds very little to the understanding it should be greyed out and perhaps marked as "[about to enter]"

Changed 4 years ago by stephan

r21619 contains the OTRE-part implementation that resolves these:

  • map interesting calls within the chaining wrapper to the base method start position -- OTRE contribution to item 1 above.
  • avoid a debug exception that was worked around in r17936 (need correct local variables table including "this")
  • fixed out-of-order generated line number infos, which caused the debugger to fully resume during stepping (couldn't get hold of the next stepping position?)

TODO: check usage of SourceMapGeneration, may be able to reduce code here.

Changed 4 years ago by stephan

r21620 brings the compiler part of the implementation, mainly:

  • remove usage of playedBy position for lift calls.

plus some minor cleanup.

r21622 implements the UI part regarding items (1) and (3) above plus removing a workaround that has become obsolete.

Pending real-life tests and synchronization with the user guide the main issues have been fixed.

Remaining:

  • a TODO regarding code cleanness from comment:2
  • a few unnecessary F5 need investigation
  • the trick using "step into selection" (comment:1) works for one callin of the flight bonus example (Item.earnCredit <- after book) but fails for another (Subscriber.buy <- replace book) -> should investigate.

Changed 4 years ago by stephan

The user guide (r21625) is in sync with the current implementation.

Changed 4 years ago by stephan

r21635 avoids the need for an extra F5 after evaluating a guard predicate.

Changed 4 years ago by stephan

Advances from r21638:

Remove class SourceMapGeneration as suggested in comment:2.

Make stepping behavior configurable using a new property "ot.debug.callin.stepping", which accepts a comma-separated list of the tokens "role", "recurse" and "orig".

For each token in the list the debugger will stop at the corresponding location, position the editor at the base method header and display "{{Dispatching callins for bm}}". An explicit F5 (step-into) is needed to enter the corresponding action.

The individual tokens stand for

  • role: invocation of the callin, i.e., jumping to the callin method binding
  • recurse: performing the next step in the dispatch algorithm which is realized by recursion of the generated base method.
  • orig: invocation of the original bound base method, either due to no (more) team instances, or via a base call.

Not setting the property is setting it to the value "role,recurse,orig". Hiding all these locations (causing automatic step-into) can be achieved by assigning the value "none" (or in fact any non-interpreted value).

Changed 4 years ago by stephan

  • status changed from new to closed
  • resolution set to fixed

r21634 improves the heuristic for detecting initial wrappers (to not confuse them with base call surrogates that are in fact recognized by their name already).

correction re comment:5: guard evaluation is hidden by r21636 (not -35).

The stepping behavior is now configurable via a new preference option (r21638, r21640, r21641, r21642 (user guide)).

Thus, only step-into-selection remains unresolved, for which I opened #260.

Note: See TracTickets for help on using tickets.