Closed Bug 1470558 Opened 2 years ago Closed 5 months ago

Debugger: provide detailed completion values for generator and async function suspensions

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: jorendorff, Assigned: jimb)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [debugger-mvp])

Attachments

(9 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
When an async function call frame `return`s or `await`s, the debugger currently has no way to tell which.

Here's a script to dump the completion value passed to `onPop` when we call a simple async function:

----
let g = newGlobal();
g.eval(`
    async function f() {
        await "promise";
        return "done";
    }
`);

let dbg = new Debugger(g);
dbg.onEnterFrame = frame => {
    frame.onPop = completion => { print(uneval(completion)); };
};

g.f();
drainJobQueue();
----

The output is three lines:

----
({return:{}})
({return:"promise"})
({return:"done"})
----

The first `onPop` fires during the async function's "initial suspend". (I don't understand why we have to do this for async functions.) The return value is a Generator that's an implementation detail of the async function task.

The second `onPop` is for the `await`, and the third is for the `return`.
The obvious fix would be to change await completions to look like `{await: <value>}`. It might break some existing code, though. :-\
Priority: -- → P3
Blocks: 1092910
Assignee: nobody → jimb

Our ultimate goal in this bug is to include information about yields and awaits
in the completion value passed to the onPop handler. This means that detecting
when a frame is being suspended becomes just another step in building a
completion value. That change becomes a little clearer if those two steps happen
next to each other.

Depends on D24995

I want to break that last patch up into smaller steps, but comments on the overall approach are welcome.

Depends on: 1539654
Summary: Debugger: distinguish `await` completion values from `return` → Debugger: provide detailed completion values for generator and async function suspensions
Blocks: 1543225
Status: NEW → ASSIGNED
Assignee: jimb → nobody
Status: ASSIGNED → NEW
Whiteboard: [debugger-mvp]

Rebased patch queue.

Assignee: nobody → jimb
Status: NEW → ASSIGNED

Replacing more calls to Debugger::resultToCompletion with uses of the newer API.

DebuggerFrame::eval and DebuggerObject::evalInGlobal are two more invocation
functions that can reasonably use Completion to report the results of the
operation.

Depends on D33076

All uses of these have been rewritten to use the Completion type.

Depends on D33078

Attachment #9053780 - Attachment description: Bug 1470558: js::Debugger::slowPathOnLeaveFrame should always get a pc for non-WASM frames. → Bug 1470558: js::Debugger::slowPathOnLeaveFrame should always get a pc for non-WASM frames. r?jorendorff
Attachment #9053781 - Attachment description: Bug 1470558: Debugger::slowPathOnLeaveFrame: move 'suspending' block a little later. → Bug 1470558: Debugger::slowPathOnLeaveFrame: move 'suspending' block a little later. r?jorendorff
Attachment #9053957 - Attachment description: Bug 1470558: js/src/vm/Debugger.h: Move forward declarations to top. → Bug 1470558: js/src/vm/Debugger.h: Move forward declarations to top. r?jorendorff
Attachment #9053782 - Attachment description: Bug 1470558: Distinguish yields and awaits in completion values. → Bug 1470558: Distinguish yields and awaits in completion values. r?jorendorff
Attachment #9053958 - Attachment description: Bug 1470558: Use Completion type in DebuggerObject::{set,get}Property. → Bug 1470558: Use Completion type in DebuggerObject::{set,get}Property. r?jorendorff
Attachment #9068518 - Attachment description: Bug 1470558: Use Completion type in Debugger::receiveCompletionValue. → Bug 1470558: Use Completion type in Debugger::receiveCompletionValue. r?jorendorff
Attachment #9068519 - Attachment description: Bug 1470558: Use Completion type for Debugger eval-related methods. → Bug 1470558: Use Completion type for Debugger eval-related methods. r?jorendorff
Attachment #9068520 - Attachment description: Bug 1470558: Use Completion in Debugger.Object 'call' and 'apply' methods. → Bug 1470558: Use Completion in Debugger.Object 'call' and 'apply' methods. r?jorendorff
Attachment #9068521 - Attachment description: Bug 1470558: Delete Debugger::{resultToCompletion,newCompletionValue,receiveCompletionValue}. → Bug 1470558: Delete Debugger::{resultToCompletion,newCompletionValue,receiveCompletionValue}. r?jorendorff
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1248eb8d570b
js::Debugger::slowPathOnLeaveFrame should always get a pc for non-WASM frames. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/69a369e2cdaa
Debugger::slowPathOnLeaveFrame: move 'suspending' block a little later. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/bb62c9157f04
js/src/vm/Debugger.h: Move forward declarations to top. r=jorendorff
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Marking leave-open as there are patches that still need to be land.

Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED

Oh dear. Completion can't be passed directly as a parameter. I'll need to rework some of these interfaces.

Depends on: 1563065

If we can land bug 1563065, the interfaces can remain unchanged.

Blocks: dbg-70
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c4bae8a5a111
Distinguish yields and awaits in completion values. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/abdfe0ed8ea1
Use Completion type in DebuggerObject::{set,get}Property. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/6791d671e6cc
Use Completion type in Debugger::receiveCompletionValue. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/6034237161dc
Use Completion type for Debugger eval-related methods. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/ef67e1b47f9c
Use Completion in Debugger.Object 'call' and 'apply' methods. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/1a18ccfcdc98
Delete Debugger::{resultToCompletion,newCompletionValue,receiveCompletionValue}. r=jorendorff
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f0aa53a43409
Distinguish yields and awaits in completion values. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/3f47b78ffdc0
Use Completion type in DebuggerObject::{set,get}Property. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/f82f7c40443d
Use Completion type in Debugger::receiveCompletionValue. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/4f26d027343a
Use Completion type for Debugger eval-related methods. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/3800d671c64e
Use Completion in Debugger.Object 'call' and 'apply' methods. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/483d687212fb
Delete Debugger::{resultToCompletion,newCompletionValue,receiveCompletionValue}. r=jorendorff
Status: ASSIGNED → RESOLVED
Closed: 6 months ago5 months ago
Flags: needinfo?(jimb)
Keywords: leave-open
Resolution: --- → FIXED
Regressions: 1564230
Regressions: 1565278
Regressions: 1576862
You need to log in before you can comment on or make changes to this bug.