Nate Biggs
8bd3690ab0
This is a reland of commit d84f908641
Includes fixes:
--- Catch scope fix ---
Each catch block should define its own scope, but the compiler was not treating the body of these catch blocks as a scope. This was leading to incorrect variable renaming.
--- Duplicate label fix ---
Labels weren't being removed from the _labelNames Map after being used so multiple nodes were getting tagged with the same label. Note: This didn't actually cause any bugs because the label closer to the break statements using those labels was the correct one.
--- addAsyncCallback cleanup ---
The new async semantics actually don't require managing addAsyncCallback and removeAsyncCallback calls (proxies for async_helper.asyncStart and async_helper.asyncEnd). None of the other backends manage these themselves. It's up to tests to ensure they call async_helper.asyncStart and async_helper.asyncEnd.
Original change's description:
> [ddc] Update DDC compiler to start using the new async transform.
>
> Updates compiler.dart to use the new async transformation.
>
> Some key things to note:
> - Dart Let and BlockExpression expressions are represented as IIFEs in DDC compiled code. For non-async code this works fine but this doesn't work when they contain "await" expressions. When these expressions contain awaits we use the same lowering as we would for an async function, but instead apply it to the IIFE function. Then we simply await the IIFE Call expression as the IIFE will return a future after the transform.
> - For async/sync*/async* functions we want to make sure parameter initialization happens synchronously before any of the async logic is hit. To do this we first apply the async transform the user-code function body. We then prepend the paramter initialization logic to the body of the transformed function.
> - We add support for JS_RAW_EXCEPTION which allows the machinery in async_patch to access the wrapped JS exception in a catch block rather than the unwrapped Dart exception.
> - Stacktraces and sourcemaps have some differences. There is still room for improvement in these but they should at least allow users to reasonably step through parts of the async code.
>
>
> There are also several test fixes/updates associated with this change. The test_runner wrapper no longer has to inject in asyncStart/asyncEnd calls, these are handled by the new async logic.
>
> Change-Id: I0f9f547cd9eb52ff7d850d277876d4d57568a14e
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/374444
> Reviewed-by: Bob Nystrom <rnystrom@google.com>
> Reviewed-by: Mark Zhou <markzipan@google.com>
> Reviewed-by: Johnni Winther <johnniwinther@google.com>
> Reviewed-by: Nicholas Shahan <nshahan@google.com>
Change-Id: Idfe05c0628b2b91f474d08d99427961381debeb5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/376000
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
Testing of source maps
Currently this package consists of two "frameworks":
- stacktrace_helper.dart
- stepping_helper.dart
It is intended to be a shared resource between DDC and dart2js.
Stacktraces ("stacktrace_helper.dart")
TODO
Debugging tests (step tests, "stepping_helper.dart")
This is supposed to work in a few steps:
- Create the JS
- Run the JS with D8, setting instructed breakpoints etc.
- Translating JS positions to Dart positions
- Validating the stepped positions.
In the above, the helper assumes
- The dart file is called "test.dart" and is placed in the output directory before compiling to "js.js" in the output dir.
And then performs via
2) runD8AndStep
3) checkD8Steps
4) (done in above step)
The test files themselves contain information about where to stop, which breakpoints to expect etc.
They contain this information in comments inlined in the code as in /*key*/ where key can be one
of the below.
Not context sensitive
These comments can be anywhere in the file and their position does not matter.
- fail: Will fail the test. Useful for debugging in conjunction with debug being set to true (see below).
- Debugger:stepOver: Will step over breakpoints. Default (i.e. without this) is to step into.
Context sensitive
These comments should be placed at the wanted position: The line and possibly column position of the comment matters. They refer to the next non-whitespace position in the source.
- bl (break line): insert a breakpoint on this line. This does not add any new expected breaks.
- s:{i} (stop): adds an expected stop as the
ith stop (1-indexed). - sl:{i} (stop at line): adds an expected stop as the
ith stop (1-indexed). Only check the line number. - nb (no break): The debugger should never break on this line.
- nbc (no break column): The debugger should never break on this line and column.
- nbb:{i}:{j} (no break between): The debugger should not break on this line between expectation
iandj(1-indexed). Note thatfromcan also be the special value0meaning from the very first stop. For examplenbb:0:1means not before first expected stop. - nm (no mapping): There's not allowed to be any mapping to this line.
- bc:{i} (break column): inserts a breakpoint at this line and column and adds an expected stop
as the
ith stop (1-indexed).
Note that in an ideal world bc:{i} would not be unnecessary: Stopping at a line and stepping
should generally be enough. Because of the current behavior of d8 though, for instance
baz(foo(), bar())
will stop at baz, go into foo, stop at bar, go into bar and stop at baz.
From a Dart perspective we would instead expect it to stop at foo, go into foo, stop at bar,
go into bar and stop a baz.
Having bc:{i} allows us to force this behavior as d8 can actually stop at foo too.
All of these annotations are removed before compiling to js and the expected output thus refers to the unannotated code.
When the test confirms that the debugger stopped at the expected locations it allows for additional breakpoints before, between and after the expected breakpoints.
Debugging a test
By calling checkD8Steps with the debug parameter set to true one can get information like this
dumped to standard out:
Stop #1
test.main = function() { | main() {
try { | try {
let value = /*STOP*/"world"; | var value = /*STOP*/"world";
dart.throw(dart.str`Hello, ${value}`); | // Comment
} catch (e) { | throw "Hello, $value";
Stop #2
try { | var value = "world";
let value = "world"; | // Comment
/*STOP*/dart.throw(dart.str`Hello, ${value}`); | /*STOP*/throw "Hello, $value";
} catch (e) { | }
let st = dart.stackTrace(e); | // Comment
Stop #3
dart.throw(dart.str`Hello, ${value}`); | }
} catch (e) { | // Comment
let st = /*STOP*/dart.stackTrace(e); | catch (e, /*STOP*/st) {
{ | print(e);
core.print(e); | print(st);
[...]
This can for instance be useful in combination with /*fail*/ when adding new tests to see all the
places where the debugger stopped.
Technical details
Some of the logic comes from https://github.com/ChromeDevTools/devtools-frontend/, for instance see https://github.com/ChromeDevTools/devtools-frontend/blob/fa18d70a995f06cb73365b2e5b8ae974cf60bd3a/front_end/sources/JavaScriptSourceFrame.js#L1520-L1523 for how a line breakpoint is resolved: Basically the line asked to break on in user code (e.g. in dart code) is asked for first and last javascript positions; these are then used to get possible breakpoints in that part. If there are none it tries the next line (etc for a number of lines). Once it finds something (in javascript positions) it converts that to user code position (e.g. in dart code), normalizes it by converting to javascript position and back to user code position again, then converts to javascript position and sets the breakpoint. This is to some extend mimicked here when setting a line break (though not a "column break").