Skip to content

Conversation

@nicolo-ribaudo
Copy link

@nicolo-ribaudo nicolo-ribaudo commented Aug 21, 2025

See tc39#196 (comment).

I tried to make it a bit easier to see what is going on by:

  • giving different names to the different SDOs that take care of parsing different data about the scopes
  • getting all the data and then creating the Original Scope Record, rather than creating an Original Scope Record and then mutating it
  • centralizing the "increment the index in the state and get the result" in a single AO

We could do something similar for TopLevelItemList.

<emu-alg>
1. Perform DecodeScopesInfoItem of |OriginalScopeTreeList| with arguments _info_, _state_ and _names_.
1. Let _originalScopes_ be the DecodedOriginalScopeTrees of |OriginalScopeTreeList| with arguments _state_ and _names_.
1. Set _info_.[[Scopes]] to _originalScopes_.
Copy link
Owner

Choose a reason for hiding this comment

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

Since we need to set the result on DecodeOriginalScopeTrees now on _info_.[[Scopes]], do we also need to duplicate this for the production:

Scopes :
  OriginalScopeTreeList

Since the application of the chain rule only performs the SDO but does not set t on _info_?

spec.emu Outdated
_state_: a Decode Scope State Record,
_names_: a List of Strings,
)
): a List of Original Scope Record
Copy link
Owner

Choose a reason for hiding this comment

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

It is important that we preserve the OriginalScopeTreeItem: [empty] case with a null.

The motivation is that if a source map has a list of N sources, then we expect N original scope trees. To signify that a specific source file doesn't have an original scope tree, we currently use the empty production and add null.

The PR currently removes the null, so afterwards we can't associate source records with their respective original scope tree.

I don't have a strong opinion if we want to do this differently. Either by having an "empty" tag, or by adding a "sourceFileIndex" field or item.

If we keep the "null" route we'd need to:

  • Return here a List of either a Original Scope Record or ~null~
  • Change the empty production to return a single element list: 1. Return « ~null~ »

Copy link
Author

Choose a reason for hiding this comment

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

Should we also add null to the flattened scopes?

Copy link
Owner

Choose a reason for hiding this comment

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

I'd say no. Reason being that the "definition index" in a generated range is basically the Nth "original scope start" item. I.e. an index into the flattened scopes without pushing the top-level null scopes.

@nicolo-ribaudo nicolo-ribaudo force-pushed the refactor-scopes-decoding branch from e9babd7 to 7479908 Compare August 28, 2025 16:54
@nicolo-ribaudo
Copy link
Author

I have an extra closing tag somewhere, that's why the diff is weird.

@nicolo-ribaudo nicolo-ribaudo force-pushed the refactor-scopes-decoding branch from 7479908 to 0043d9c Compare August 28, 2025 17:04
@nicolo-ribaudo
Copy link
Author

Fixed!

@nicolo-ribaudo
Copy link
Author

I'll re-open on the other repo

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.

2 participants