Skip to content

Find best locations for breakpoints, expression evaluation, and call frames. #1590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

annagrin
Copy link
Contributor

@annagrin annagrin commented Apr 21, 2022

Our current heuristics for finding the best Dart and JS locations for breakpoints, expression evaluation, and call frames are not giving correct results in some cases:

  • Stepping into a call maps into incorrect line in dart if the first line compiles into a a call expression saved into a temp.
  • Expression evaluation fails in the case above due to failing to find location.

Update heuristics to account for those cases, and use them consistently.

Closes: #880

@annagrin annagrin requested review from elliette and nshahan April 21, 2022 00:53
Copy link
Contributor

@nshahan nshahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any tests for what location is found among possible breakpoints? I'm starting to see that we could accidentally break this logic by adding or removing source mappings in DDC.

@nshahan
Copy link
Contributor

nshahan commented Apr 21, 2022

Were there any cases where additional source mapped locations would produce a more accurate breakpoint? We could update DDC to produce better source maps if needed.

@annagrin
Copy link
Contributor Author

annagrin commented Apr 21, 2022

@nshahan

Were there any cases where additional source mapped locations would produce a more accurate breakpoint? We could update DDC to produce better source maps if needed.

Yes, for example, in all cases below there is only one location starting at main. (Not sure if adding new locations will cause any performance hits though, especially for large source maps, so it might require measurements):

  • t33 = main.doSomething() in top frame:
    Current column is at t33. We return existing location starting at main. Adding a source map location at t33 will help us find the exact location for the frame.

  • main.doSomething() in top frame:
    Current column is at main. Return existing location starting at main. No problem here.

  • main.doSomething() in a frame down the stack:
    Current column is at doSomething. Source map does not have a location stored that starts at doSomething(). We return existing location starting at main. Adding a source map location at doSomething() will help us find a precise location for for the frame.

In all those cases the heuristic works for now, and adding locations would just make the heuristics return precise match in dart instead of dart location mapped to the start of main. We might not see significant differences in the UI though as it usually marks the whole line.

Update: there is one case I found where adding a location would benefit the debugger (both chrome and dwds):

dart-lang/sdk#48874

@annagrin annagrin requested review from nshahan and elliette April 25, 2022 17:26
Copy link
Contributor

@nshahan nshahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the test cases!

@annagrin
Copy link
Contributor Author

@nshahan do we have sourcemap test cases in the ddc code for cases like that as well? We could benefit on having them early in our chain of dependencies.

@annagrin annagrin merged commit 66bba1d into dart-lang:master Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support evaluation at location that don't precisely match known dart location
3 participants