Skip to content
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

Scopes: Re-purpose isScope as isFunctionScope for stack trace processing #114

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

szuend
Copy link
Collaborator

@szuend szuend commented Jul 8, 2024

This PR renames isScope to isFunctionScope. The intention is to mark each generated range that is also something callable (functions, methods, arrow functions, accessors, constructors, ...). The purpose is twofold:

  1. Stack trace processors can use this to figure out which call frames to show. The check basically becomes range.isFunctionScope && !range.hasDefinition. See Scopes: support for hiding new injected scopes from the stack trace #113 for a detailed discussions and also why a isHidden flag is not sufficient.
  2. It makes inlining easier to figure out for stack trace processors: With this flag they can tell exactly at which generated range to stop expanding inlined frames.

Note that for 1. in particular this is a great boon for generators. If we don't have this flag then generators have to "mask" callsites for "hidden functions" instead. Which is a lot trickier especially if not all call-sites are known by a generator. Again, see #113 for an example/discussion.

@szuend szuend changed the title [scopes] Repurpose isScope as isFunctionScope for stack trace processing Scopes: Re-purpose isScope as isFunctionScope for stack trace processing Jul 8, 2024
@jkup
Copy link
Collaborator

jkup commented Aug 14, 2024

@hbenl @takikawa @jridgewell Any thoughts on this one? I wasn't sure if it was blocking progress on the Scopes proposal implementations or not!

@jridgewell
Copy link
Member

The change seems fine, but I don't really have an opinion one way or the other because it doesn't affect the build-tooling side (only the consuming side). If @szuend thinks this makes consuming the ranges easier for dev-tools, then let's merge.

copybara-service bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this pull request Sep 6, 2024
This CL aligns DevTools with an in-flight (but approved) spec PR:
tc39/source-map#114. We need the flag to
properly hide stack frames from stack traces that got inserted
by transpilers and don't have a corresponding function in authored
code.

The flag also helps when dealing with inlining.

R=kimanh@chromium.org

Bug: 40277685
Change-Id: Iafef35e6ccdb0acf56b2fa91aafbdb44f03c6bce
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5836934
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
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.

3 participants