Skip to content

Scopes: Use unsigned VLQ (1-based) for binding expressions #205

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
merged 1 commit into from
May 14, 2025

Conversation

szuend
Copy link
Collaborator

@szuend szuend commented Apr 25, 2025

As discussed in the meeting of 2025-04-24, this PR changes the binding expressions from a signed VLQ to a unsigned VLQ. To signify a variable is unavailable in a given generated (sub) range, we use 0 instead of -1 and the indices themselves are now 1-based instead of 0-based. This is necessary to free up 0 as the "unavailability marker".

To capture some of the points of the meeting here (also available in the notes):

  • Using relative indicies into the names array would require us to sacrifice an additional bit to signify "unavailability".
  • Another option would be to use 0 as the marker and make positive relative indices "off by one". E.g. "0" means a variable is unavailable, "1" means a variable uses the same binding expression as the previous variable and "2" means it uses the next entry in "names".
  • Both of the above options show some minor improvement in the uncompressed size of the scopes encoding but are actually worse after gzip/brotli compression.
  • This leads us to this PR as to not overcomplicate the encoding for no gain.

@szuend szuend requested a review from hbenl April 25, 2025 05:09
@szuend
Copy link
Collaborator Author

szuend commented Apr 25, 2025

@nicolo-ribaudo adding you explicitly since you didn't have a chance yet to express your opinion. I remember you floated the idea of using B (the minimum possible integer) to mark "unavailability" of a variables' value. We didn't discuss that option though in yesterdays' meeting.

@szuend szuend requested a review from nicolo-ribaudo April 25, 2025 05:18
@szuend szuend merged commit e4dba00 into tc39:main May 14, 2025
3 checks passed
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