Skip to content

feat(data_structures): introduce NonNullConst and NonNullMut pointer types#21425

Merged
graphite-app[bot] merged 1 commit intomainfrom
om/04-13-feat_data_structures_introduce_nonnullconst_and_nonnullmut_pointer_types
Apr 14, 2026
Merged

feat(data_structures): introduce NonNullConst and NonNullMut pointer types#21425
graphite-app[bot] merged 1 commit intomainfrom
om/04-13-feat_data_structures_introduce_nonnullconst_and_nonnullmut_pointer_types

Conversation

@overlookmotel
Copy link
Copy Markdown
Member

@overlookmotel overlookmotel commented Apr 14, 2026

Most of our low-level data structures e.g. Arena, Vec, Box, and Stack types use NonNull pointers.

NonNull is fundamentally dangerous. A NonNull can be created from a &T, and so only has read permissions. Yet you can call as_mut() on such a pointer to gain a &mut T, and that's instant undefined behavior. The onus is on the author to manually track how pointers were created and so whether they have write permissions or not, to avoid this hazard.

In complex code like the arena allocator, pointers are created in various different places, stored in structs, and passed around, so it is not trivial to track their permissions, and neither the type system nor Clippy gives you any help.

In comparison, raw pointers *const T and *mut T do make such a distinction between read-only and write-enabled pointers, but they have other shortcomings - they lack the non-null guarantee of NonNull (which can improve perf in some cases), and *mut T is not covariant, so it is unsuitable for use in types like Vec.

This PR intoduces 2 new pointer types NonNullConst and NonNullMut. Both are just wrappers around NonNull and are equivalent to NonNull at runtime. What they add is type system-level guarantees of the read/write permissions that these pointers have. i.e. they bring together the const/mut distinction of raw *const / *mut pointers, and the advantages of NonNull.

This PR only introduces these new pointer types, but does not use them in the codebase yet. It is my intention to migrate Arena allocator over to these new pointer types, to make it harder to blunder into causing UB while reworking the implementation. Other types like Vec, Box, Stack, and lexer's Source could also be migrated over later on.

Copy link
Copy Markdown
Member Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions Bot added the C-enhancement Category - New feature or request label Apr 14, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 14, 2026

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing om/04-13-feat_data_structures_introduce_nonnullconst_and_nonnullmut_pointer_types (9bac6e5) with main (091e88e)

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@overlookmotel overlookmotel self-assigned this Apr 14, 2026
@overlookmotel overlookmotel marked this pull request as ready for review April 14, 2026 13:19
Copilot AI review requested due to automatic review settings April 14, 2026 13:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Introduces new non-null pointer wrapper types to encode read-only vs read-write permissions in the type system, aiming to reduce accidental UB compared to using std::ptr::NonNull<T> directly.

Changes:

  • Adds NonNullConst<T> and NonNullMut<T> wrappers (plus supporting APIs/traits and tests) under a new non_null feature.
  • Wires the new module behind a crate feature flag and includes it in the crate’s all feature.
  • Updates crate README to mention the new pointer utilities.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
crates/oxc_data_structures/src/non_null.rs Adds NonNullConst/NonNullMut, supporting APIs (slice/array helpers, sealed IntoNonNull), and unit tests.
crates/oxc_data_structures/src/lib.rs Exposes the non_null module behind the non_null feature.
crates/oxc_data_structures/README.md Documents availability of the new non-null pointer types.
crates/oxc_data_structures/Cargo.toml Adds the non_null feature and includes it in the all feature set.

Comment thread crates/oxc_data_structures/src/non_null.rs
Comment thread crates/oxc_data_structures/src/non_null.rs Outdated
@overlookmotel overlookmotel force-pushed the om/04-13-feat_data_structures_introduce_nonnullconst_and_nonnullmut_pointer_types branch from 12e5a7b to 9bac6e5 Compare April 14, 2026 13:31
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Apr 14, 2026
Copy link
Copy Markdown
Member Author

overlookmotel commented Apr 14, 2026

Merge activity

…ter types (#21425)

Most of our low-level data structures e.g. `Arena`, `Vec`, `Box`, and `Stack` types use `NonNull` pointers.

`NonNull` is fundamentally dangerous. A `NonNull` can be created from a `&T`, and so only has read permissions. Yet you can call `as_mut()` on such a pointer to gain a `&mut T`, and that's instant undefined behavior. The onus is on the author to manually track how pointers were created and so whether they have write permissions or not, to avoid this hazard.

In complex code like the arena allocator, pointers are created in various different places, stored in structs, and passed around, so it is not trivial to track their permissions, and neither the type system nor Clippy gives you any help.

In comparison, raw pointers `*const T` and `*mut T` *do* make such a distinction between read-only and write-enabled pointers, but they have other shortcomings - they lack the non-null guarantee of `NonNull` (which can improve perf in some cases), and `*mut T` is not covariant, so it is unsuitable for use in types like `Vec`.

This PR intoduces 2 new pointer types `NonNullConst` and `NonNullMut`. Both are just wrappers around `NonNull` and are equivalent to `NonNull` at runtime. What they add is type system-level guarantees of the read/write permissions that these pointers have. i.e. they bring together the const/mut distinction of raw `*const` / `*mut` pointers, and the advantages of `NonNull`.

This PR only introduces these new pointer types, but does not use them in the codebase yet. It is my intention to migrate `Arena` allocator over to these new pointer types, to make it harder to blunder into causing UB while reworking the implementation. Other types like `Vec`, `Box`, `Stack`, and lexer's `Source` could also be migrated over later on.
@graphite-app graphite-app Bot force-pushed the om/04-13-feat_data_structures_introduce_nonnullconst_and_nonnullmut_pointer_types branch from 9bac6e5 to 24b03de Compare April 14, 2026 13:42
@graphite-app graphite-app Bot merged commit 24b03de into main Apr 14, 2026
27 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Apr 14, 2026
@graphite-app graphite-app Bot deleted the om/04-13-feat_data_structures_introduce_nonnullconst_and_nonnullmut_pointer_types branch April 14, 2026 13:46
Dunqing added a commit that referenced this pull request Apr 16, 2026
### 💥 BREAKING CHANGES

- 24fb7eb allocator: [**BREAKING**] Rename `Box` and `Vec` methods
(#21395) (overlookmotel)

### 🚀 Features

- ce5072d parser: Support `turbopack` magic comments (#20803) (Kane
Wang)
- f5deb55 napi/transform: Expose `optimizeConstEnums` and
`optimizeEnums` options (#21388) (Dunqing)
- 24b03de data_structures: Introduce `NonNullConst` and `NonNullMut`
pointer types (#21425) (overlookmotel)

### 🐛 Bug Fixes

- d7a359a ecmascript: Treat update expressions as unconditionally
side-effectful (#21456) (Dunqing)
- 56af2f4 transformer/async-to-generator: Correct scope of inferred
named FE in async-to-generator (#21458) (Dunqing)
- b3ed467 minifier: Avoid illegal `var;` when folding unused arguments
copy loop (#21421) (fazba)
- b0e8f13 minifier: Preserve `var` inside `catch` with same-named
parameter (#21366) (Dunqing)
- 4fb73a7 transformer/typescript: Preserve execution order for accessor
with `useDefineForClassFields: false` (#21369) (Dunqing)

### ⚡ Performance

- da3cc16 parser: Refactor out `LexerContext` (#21275) (Ulrich Stark)

### 📚 Documentation

- c5b19bb allocator: Reformat comments in `Arena` (#21448)
(overlookmotel)
- 091e88e lexer: Update doc comment about perf benefit of reading
through references (#21423) (overlookmotel)
- 922cbee allocator: Remove references to "bump" from comments (#21397)
(overlookmotel)

Co-authored-by: Dunqing <29533304+Dunqing@users.noreply.github.com>
graphite-app Bot pushed a commit that referenced this pull request Apr 22, 2026
`just doc` run rustdoc with `--all-features` flag.

I discovered that `just doc` was not, for example, catching errors in doc comments in #21425, because the code is behind a feature which is not enabled by default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants