Skip to content

Conversation

@nicolo-ribaudo
Copy link
Member

Originally opened at szuend#1. See #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.

@nicolo-ribaudo nicolo-ribaudo changed the title Refactor scopes tree decoding Scopes: Refactor scopes tree decoding Aug 28, 2025
@nicolo-ribaudo
Copy link
Member Author

I just noticed that this makes FlattenedScopes in post-order rather than pre-order, I need to fix that.

1. If |OriginalScopeVariablesItem| is present, then
1. Set _originalScope_.[[Variables]] to the OriginalScopeVariables of |OriginalScopeVariablesItem| with arguments _state_.[[ScopeVariableIndex]] and _names_.
1. If |OriginalScopeItemList| is present, then
1. Set _originalScope_.[[Children]] to the DecodedOriginalScopeTrees of |OriginalScopeItemList| with arguments _state_ and _names_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be DecodeInnerOriginalScopeTrees?

1. Let _childScope_ be DecodeOriginalScope(|OriginalScopeTree|, _state_, _names_).
1. Append _childScope_ to _scope_.[[Children]].
1. Let _left_ be the DecodedOriginalScopeTrees of |OriginalScopeItemList| with arguments _state_ and _names_.
1. Let _right_ be the DecodedOriginalScopeTrees of |OriginalScopeItem| with arguments _state_ and _names_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also here probably DeocdeInnerOriginalScopeTrees.

@szuend
Copy link
Collaborator

szuend commented Sep 2, 2025

Thanks! lgtm % a minor bug.

As a suggestion: We could flip the naming. So instead of having DecodeOriginalScopeTree and DecodeInnerOriginalScopeTree, we could do DecodeTopLevelOriginalScopeTree and DecodeOriginalScopeTree. It fits slightly better since we "execute" DecodeInnerOriginalScopeTree even for a top-level scope.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Sep 2, 2025

Fixed and renamed. Merging! :)

@nicolo-ribaudo nicolo-ribaudo merged commit 5e9cc41 into tc39:proposal-scopes Sep 2, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants