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

Consolidate string compare throughout #10335

Open
mhofman opened this issue Oct 24, 2024 · 0 comments
Open

Consolidate string compare throughout #10335

mhofman opened this issue Oct 24, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@mhofman
Copy link
Member

mhofman commented Oct 24, 2024

Describe the bug

We have different ways of comparing strings, none of which are "correct". Now that's we've updated to a version of endo with endojs/endo#2008, we can use its compareByCodePoints throughout.

To Reproduce

Incorrect string comparison

  • a < b or any variation. This results in a comparison by UTF-16 code units.
  • [a, b].sort() similarly uses code units
  • a.localeCompare() compares by environment specific local rules. The SES shim is supposed to tame this by default, but I have seen surprising results (see Consistent store order #10299 test and undo localeCompare change).

Expected behavior

Comparison by codepoints which we're trying to standardize on, and is the behavior for any implementation that compares utf-8 encoded bytes like SQLite, golang (doesn't strictly requires that string be utf-8, but literals and range iterations are), and JSON (which specifies utf-8 as the default encoding).

Additional context

#10299 fixes the more egregious of these cases, which caused a discrepancy in the order of Stores when they're heap or virtual/durable, and whether they're in a real or fake liveslots environment. In comments, @gibson042 also points out at other cases of mistaken comparisons (likely not exhaustive).

@mhofman mhofman added the bug Something isn't working label Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant