-
Benedikt Meurer authored
Be explicit about source positions for `Return`s in the BytecodeGenerator, and only do self-healing explicitly in the `ReturnStatement` translation, where an end position of `kNoSourcePosition` is turned into the return position of the function literal. This allows us to reason more easily about which `Return`s actually receive a meaningful source position, and in particular it allows us to construct the internal `Return`s for `yield` and `yield*` with no source position attached to them. Previously they'd get the source position for the implicit (final) return attached to it, which confused the debugger and led to breakpoints being set in the completely wrong spot. Considering the simplified example ``` function* foo(){ var a = 1; } ``` this would previously generate the following bytecode ``` 0 : SwitchOnGeneratorState r0, [0], [1] { 0: @20 } 4 : Mov <closure>, r2 7 : Mov <this>, r3 13 E> 10 : InvokeIntrinsic [_CreateJSGeneratorObject], r2-r3 14 : Star0 13 E> 15 : SuspendGenerator r0, r0-r1, [0] 20 : ResumeGenerator r0, r0-r1 24 : Star2 25 : InvokeIntrinsic [_GeneratorGetResumeMode], r0-r0 29 : SwitchOnSmiNoFeedback [1], [2], [0] { 0: @39, 1: @36 } 33 : Ldar r2 13 E> 35 : Throw 36 : Ldar r2 30 S> 38 : Return <=========================== internal Return 27 S> 39 : LdaSmi [1] 41 : Star1 42 : LdaUndefined 30 S> 43 : Return ``` where everything between offset 4 and 42 corresponds to the implicit yield at the beginning of every generator function, in particular the code between 20 and 42 corresponds to that initial yields resumption logic. Notice how the internal Return at offset 38 gets assigned the source position of the function literal (the same as the implicit return at the end). This confuses the debugger quite a bit when trying to set a breakpoint on the closing brace, since it's going in bytecode order and will thus discover the `Return` at offset 38 first (matching the source position 30 it's currently looking for) and setting the breakpoint there. This `Return` bytecode however is only executed when the generator is resumed via `GeneratorPrototype.return()`, and it'll not hit when the developer uses the generator normally, which is not the desired behavior and extremely confusing (especially since stepping on the other hand works as expected). With this patch, we no longer slap a source position (and in particular not the function literal's return position) onto these internal `Return`s as you can see from the generated bytecode below: ``` 0 : SwitchOnGeneratorState r0, [0], [1] { 0: @20 } 4 : Mov <closure>, r2 7 : Mov <this>, r3 13 E> 10 : InvokeIntrinsic [_CreateJSGeneratorObject], r2-r3 14 : Star0 13 E> 15 : SuspendGenerator r0, r0-r1, [0] 20 : ResumeGenerator r0, r0-r1 24 : Star2 25 : InvokeIntrinsic [_GeneratorGetResumeMode], r0-r0 29 : SwitchOnSmiNoFeedback [1], [2], [0] { 0: @39, 1: @36 } 33 : Ldar r2 13 E> 35 : Throw 36 : Ldar r2 38 : Return 27 S> 39 : LdaSmi [1] 41 : Star1 42 : LdaUndefined 30 S> 43 : Return ``` This also allows us to remove the break position finding hack that was kept in BreakIterator::BreakIndexFromPosition() for generators and modules. Fixed: chromium:901819 Change-Id: If19a6b26e2622d49b6b5e54bf7a162747543f970 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2727820Reviewed-by: Yang Guo <yangguo@chromium.org> Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#73119}
06359f74