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

core: Improve ergonomics by replacing GcContext by StringContext #18105

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

moulins
Copy link
Contributor

@moulins moulins commented Sep 29, 2024

Supercedes #18081.

Embeds a &Mutation<'gc> directly into the string interner context (which is renamed from GcContext to StringContext, for clarity), and makes it accessible in UpdateContext without the need for an explicit reborrow method.

This simplifies most usages, as we don't need to pass a &Mutation<'gc> separately.

@moulins moulins added A-core Area: Core player, where no other category fits T-refactor Type: Refactor / Cleanup labels Sep 29, 2024
This is essentially still the same type, but with these differences:
- It is explicitely intended to be used for `AvmString` manipulation
- It is held inside `UpdateContext` instead of being created on-demand,
  making explicit 'reborrowing' methods unncessary.

Note the `unsafe` lifetime extension in `PlayerBuilder::create_gc_root`,
which is sound for mostly the same reasons as `Player::enter_arena(_mut)`
This makes usage much simpler: `context.intern(...)` instead of
`context.interner.intern(context.gc(), ...)`.

Also make `StringContext.interner` private, as it doesn't need to be
publicly accessible anymore.
- Rename `get_char` to `make_char`, and `get_ascii_char` to `ascii_char`:
  this makes clearer that only `make_char` may allocate.
- Take a `u8` instead of a `char` in `ascii_char`.
- Take a `Range<usize>` instead of two `usize` args in `substring`.
@moulins moulins enabled auto-merge (rebase) October 2, 2024 01:19
@moulins moulins merged commit 9cc1406 into ruffle-rs:master Oct 2, 2024
17 checks passed
@moulins moulins deleted the string-context branch October 2, 2024 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core player, where no other category fits T-refactor Type: Refactor / Cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants