Start using pattern types in libcore#136006
Open
oli-obk wants to merge 1 commit intorust-lang:mainfrom
Open
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
rustbot
added
S-waiting-on-review
oli-obk
added
S-blocked
This comment has been minimized.
This comment has been minimized.
oli-obk
force-pushed
the
push-tzonluoyuwkq
branch
from
e3e2cbd to
ff4d569
Compare
compiler-errors
approved these changes
Jan 24, 2025
Contributor
compiler-errors
left a comment
compiler-errors
left a comment
There was a problem hiding this comment.
Well r=me when/if it does get unblocked.
This comment has been minimized.
This comment has been minimized.
scottmcm
reviewed
Jan 25, 2025
oli-obk
force-pushed
the
push-tzonluoyuwkq
branch
from
ff4d569 to
0409353
Compare
This comment has been minimized.
This comment has been minimized.
oli-obk
force-pushed
the
push-tzonluoyuwkq
branch
from
0409353 to
4db40b9
Compare
Collaborator
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
Member
|
Re rust-analyzer blocker, as it turns out to implement pattern types (to a degree that would unblock this) it comes down to the same architecture changes required as for rust-lang/rust-analyzer#7434 (which I am currently looking into), so that will take a bit. |
This comment has been minimized.
This comment has been minimized.
oli-obk
force-pushed
the
push-tzonluoyuwkq
branch
from
4db40b9 to
0afd16b
Compare
This comment has been minimized.
This comment has been minimized.
oli-obk
force-pushed
the
push-tzonluoyuwkq
branch
from
0afd16b to
ed90eb8
Compare
This comment has been minimized.
This comment has been minimized.
oli-obk
force-pushed
the
push-tzonluoyuwkq
branch
from
ed90eb8 to
9514a86
Compare
This comment has been minimized.
This comment has been minimized.
oli-obk
force-pushed
the
push-tzonluoyuwkq
branch
from
9514a86 to
9774687
Compare
This comment has been minimized.
This comment has been minimized.
Collaborator
|
Some changes occurred to the CTFE machinery cc @rust-lang/wg-const-eval Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri |
This comment has been minimized.
This comment has been minimized.
RalfJung
reviewed
Jan 27, 2025
rust-bors bot
pushed a commit
that referenced
this pull request
Feb 12, 2026
Start using pattern types in libcore (NonZero and friends) part of #136006 This PR only changes the internal representation of `NonZero`, `NonMax`, ... and other integral range types in libcore. This subsequently affects other types made up of it, but nothing really changes except that the field of `NonZero` is now accessible safely in contrast to the `rustc_layout_scalar_range_start` attribute, which has all kinds of obscure rules on how to properly access its field.
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Feb 13, 2026
Start using pattern types in libcore (NonZero and friends) part of rust-lang/rust#136006 This PR only changes the internal representation of `NonZero`, `NonMax`, ... and other integral range types in libcore. This subsequently affects other types made up of it, but nothing really changes except that the field of `NonZero` is now accessible safely in contrast to the `rustc_layout_scalar_range_start` attribute, which has all kinds of obscure rules on how to properly access its field.
oli-obk
force-pushed
the
push-tzonluoyuwkq
branch
from
875e95d to
0bfe9db
Compare
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rust-bors bot
pushed a commit
that referenced
this pull request
Feb 13, 2026
Start using pattern types in libcore
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
oli-obk
force-pushed
the
push-tzonluoyuwkq
branch
from
0bfe9db to
f892c20
Compare
Contributor
This comment has been minimized.
This comment has been minimized.
Collaborator
|
Finished benchmarking commit (224f039): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.5%, secondary -2.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.5%, secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.1%, secondary 0.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 482.691s -> 481.225s (-0.30%) |
rustbot
removed
the
S-waiting-on-perf
Contributor
Author
|
Hell yea @rustbot ready r? compiler |
rustbot
added
S-waiting-on-review
scottmcm
reviewed
Feb 14, 2026
| // Remember to use `.as_ptr()` instead of `.pointer`, as field projecting to | ||
| // this is banned by <https://github.com/rust-lang/compiler-team/issues/807>. | ||
| pointer: *const T, | ||
| pointer: crate::pattern_type!(*const T is !null), |
Member
There was a problem hiding this comment.
I'm excited to do this, but it feels odd to me to have both cg_ssa changes and the change to the library type in one PR.
Would it maybe be feasible to split this to have zero library changes, just enough new test coverage to justify the compiler updates, then a follow-up no-compiler-review-needed change to the library part?
Member
There was a problem hiding this comment.
hmm, I guess the box elaboration needs to be in the same PR as the library change, doesn't it :/
Contributor
Author
There was a problem hiding this comment.
Yea... I can land some changes ahead of time, but I can only test them with a custom minicore
Contributor
Author
There was a problem hiding this comment.
Gave this another try, I should be able to hit the cg_ssa changes with a test written on rustc today, but I'm missing some necessary component to actually go down this path. I tried https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=72f945c494a52dbb039b2aff32453d41 (and we already have a test that this is a permutation of: https://github.com/rust-lang/rust/blob/main/tests/ui/type/pattern_types/unsize.rs
scottmcm
reviewed
Feb 14, 2026
| _12 = NonNull::<T> { pointer: copy _14 }; | ||
| _15 = copy _14 as (*const T) is !null (Transmute); | ||
| StorageDead(_14); | ||
| _12 = NonNull::<T> { pointer: move _15 }; |
Member
There was a problem hiding this comment.
not this PR's problem: looks like we should either avoid ever using NonNull(blah) in the library because that doesn't fold in GVN or should look at allowing constructors like this to fold with transmutes.
This comment has been minimized.
This comment has been minimized.
github-actions bot
pushed a commit
to rust-lang/rust-analyzer
that referenced
this pull request
Feb 16, 2026
Start using pattern types in libcore (NonZero and friends) part of rust-lang/rust#136006 This PR only changes the internal representation of `NonZero`, `NonMax`, ... and other integral range types in libcore. This subsequently affects other types made up of it, but nothing really changes except that the field of `NonZero` is now accessible safely in contrast to the `rustc_layout_scalar_range_start` attribute, which has all kinds of obscure rules on how to properly access its field.
github-actions bot
pushed a commit
to rust-lang/rustc-dev-guide
that referenced
this pull request
Feb 16, 2026
Start using pattern types in libcore (NonZero and friends) part of rust-lang/rust#136006 This PR only changes the internal representation of `NonZero`, `NonMax`, ... and other integral range types in libcore. This subsequently affects other types made up of it, but nothing really changes except that the field of `NonZero` is now accessible safely in contrast to the `rustc_layout_scalar_range_start` attribute, which has all kinds of obscure rules on how to properly access its field.
Use `!null` pattern type in libcore
oli-obk
force-pushed
the
push-tzonluoyuwkq
branch
from
f892c20 to
58a3948
Compare
Collaborator
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Contributor
|
☔ The latest upstream changes (presumably #148190) made this pull request unmergeable. Please resolve the merge conflicts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cc #135996
Replaces the innards of
NonNullwith*const T is !null.This does affect LLVM's optimizations, as now reading the field preserves the metadata that the field is not null, and transmuting to another type (e.g. just a raw pointer), will also preserve that information for optimizations. This can cause LLVM opts to do more work, but it's not guaranteed to produce better machine code.
Once we also remove all uses of rustc_layout_scalar_range_start from rustc itself, we can remove the support for that attribute entirely and handle all such needs via pattern types