Skip to content

Make AsciiSet values instead of refs - breaking version #978

New issue

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

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

Already on GitHub? # to your account

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

Conversation

joshka
Copy link
Contributor

@joshka joshka commented Sep 27, 2024

Refs take 8 bytes, whereas the values are only 16 bytes, so there is not
a huge benefit to using references rather than values. PercentEncoding
is changed to store the AsciiSet as a value, and the functions that
previously accepted a reference now accept a value. This is a breaking
change for users who were passing a reference to AsciiSet to the
functions in the public API.

The AsciiSet consts (CONTROLS, NON_ALPHANUMERIC, etc.) are also changed
to be values.

This is an alternative to the non-breaking change in
#976

Discussion about the rationale for this is change is at
#970 (comment)

Refs take 8 bytes, whereas the values are only 16 bytes, so there is not
a huge benefit to using references rather than values. PercentEncoding
is changed to store the AsciiSet as a value, and the functions that
previously accepted a reference now accept a value. This is a breaking
change for users who were passing a reference to AsciiSet to the
functions in the public API.

The AsciiSet consts (CONTROLS, NON_ALPHANUMERIC, etc.) are also changed
to be values.

This is an alternative to the non-breaking change in
<servo#976>

Discussion about the rationale for this is change is at
<servo#970 (comment)>
Copy link

codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@710e1e7). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #978   +/-   ##
=======================================
  Coverage        ?   82.21%           
=======================================
  Files           ?       22           
  Lines           ?     3560           
  Branches        ?        0           
=======================================
  Hits            ?     2927           
  Misses          ?      633           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joshka joshka changed the title Make AsciiSet consts and fields values Make AsciiSet values instead of refs - breaking version Sep 27, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant