Lower index bounds checking to `PtrMetadata`, this time with the right fake borrow semantics 😸 #135748

compiler-errors · 2025-01-19T22:51:52Z

Change Rvalue::RawRef to take a RawRefKind instead of just a Mutability. Then introduce RawRefKind::FakeForPtrMetadata and use that for lowering index bounds checking to a PtrMetadata. This new RawRefKind::FakeForPtrMetadata acts like a shallow fake borrow in borrowck, which mimics the semantics of the old Rvalue::Len operation we're replacing.

We can then use this RawRefKind instead of using a span desugaring hack in CTFE.

cc @scottmcm @RalfJung

rustbot · 2025-01-19T22:52:01Z

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-01-19T22:52:03Z

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @vakaras

The Miri subtree was changed

cc @rust-lang/miri

scottmcm · 2025-01-19T23:14:36Z

compiler/rustc_mir_build/src/builder/expr/as_place.rs

+                // though. FIXME: Do we really *need* to count this as a use?
+                // Could partial array tracking work off something else instead?
+                self.cfg.push_fake_read(block, source_info, FakeReadCause::ForIndex, place);
+                let const_ = Const::from_ty_const(*len_const, usize_ty, self.tcx);


With things like #134371 and #134352, you might want something here to handle weirdness.

Not sure what the right way to do that is, but you could consider something like

Suggested change

let const_ = Const::from_ty_const(*len_const, usize_ty, self.tcx);

let const_ =

if let Some(err) = self.infcx.is_tainted_by_errors() {

Const::new_error(self.tcx, err)

} else {

Const::from_ty_const(*len_const, usize_ty, self.tcx)

};

(or whatever context is the right place to check for problems that we've already detected -- maybe self.tcx.dcx().has_errors() is better? No idea.)

Right, this inadvertently reapplies that bug I pointed out. I can fix that too.

Also, I don't really know what this has to do with error tainting?

Lowering this directly to a mir::Const::Ty causes ICEs even on code that's expected to pass, like when doing reads on a slice of &[(); T::ASSOC_CONST] when GCE is enabled (seetests/ui/const-generics/issues/issue-89146.rs) .

Anyways, this ICE is unrelated, and I'm going to fix it separately. I'm tracking that work in #135753. Either that one lands first, which will cause a conflict on this, or this one lands first and I'll fix the approach in this PR -- I don't think it needs to block this PR in any case.

The case that has to do with error tainting is when you get a len_const that's not actually a usize, like in the repro in #134352 where the len_const is actually a u64. There's already been an error about the type being wrong, but that doesn't keep the MIR from being very unhappy about a type mismatch after building returns a u64 constant for something that's supposed to be usize.

scottmcm

Thanks for putting this together, and so quickly too! Much appreciated.

compiler/rustc_middle/src/mir/syntax.rs

compiler/rustc_const_eval/src/interpret/step.rs

compiler/rustc_span/src/hygiene.rs

compiler/rustc_const_eval/src/check_consts/check.rs

compiler/rustc_const_eval/src/interpret/step.rs

src/tools/miri/tests/pass/disjoint-array-accesses.rs

compiler/rustc_const_eval/src/check_consts/check.rs

compiler/rustc_middle/src/mir/syntax.rs

compiler/rustc_const_eval/src/check_consts/check.rs

RalfJung

The const-checking and interpreter changes LGTM.

bors · 2025-01-26T13:01:14Z

☔ The latest upstream changes (presumably #135753) made this pull request unmergeable. Please resolve the merge conflicts.

compiler-errors · 2025-01-27T19:10:37Z

r? oli-obk

compiler/rustc_borrowck/src/type_check/mod.rs

…ptrmetadata, r=davidtwco,RalfJung" This reverts commit 122a55b.

…ake borrow

oli-obk · 2025-01-28T11:54:53Z

@bors r=RalfJung,oli-obk

bors · 2025-01-28T11:54:56Z

📌 Commit eeecb56 has been approved by RalfJung,oli-obk

It is now in the queue for this repository.

…oli-obk Lower index bounds checking to `PtrMetadata`, this time with the right fake borrow semantics 😸 Change `Rvalue::RawRef` to take a `RawRefKind` instead of just a `Mutability`. Then introduce `RawRefKind::FakeForPtrMetadata` and use that for lowering index bounds checking to a `PtrMetadata`. This new `RawRefKind::FakeForPtrMetadata` acts like a shallow fake borrow in borrowck, which mimics the semantics of the old `Rvalue::Len` operation we're replacing. We can then use this `RawRefKind` instead of using a span desugaring hack in CTFE. cc `@scottmcm` `@RalfJung`

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#133151 (Trim extra whitespace in fn ptr suggestion span) - rust-lang#133829 (Implement `AtomicT::update` & `AtomicT::try_update`) - rust-lang#135367 (Enable `unreachable_pub` lint in `alloc`) - rust-lang#135748 (Lower index bounds checking to `PtrMetadata`, this time with the right fake borrow semantics 😸) - rust-lang#135805 (Add missing allocator safety in alloc crate) - rust-lang#135886 (Document purpose of closure in from_fn.rs more clearly) - rust-lang#135961 (Fix 2/4 tests skipped by opt-dist) - rust-lang#136012 (Document powf and powi values that are always 1.0) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#135748 - compiler-errors:len-2, r=RalfJung,oli-obk Lower index bounds checking to `PtrMetadata`, this time with the right fake borrow semantics 😸 Change `Rvalue::RawRef` to take a `RawRefKind` instead of just a `Mutability`. Then introduce `RawRefKind::FakeForPtrMetadata` and use that for lowering index bounds checking to a `PtrMetadata`. This new `RawRefKind::FakeForPtrMetadata` acts like a shallow fake borrow in borrowck, which mimics the semantics of the old `Rvalue::Len` operation we're replacing. We can then use this `RawRefKind` instead of using a span desugaring hack in CTFE. cc ``@scottmcm`` ``@RalfJung``

… r=compiler-errors Implement RustcInternal for RawPtrKind Implement `RustcInternal` for `RawPtrKind`. rust-lang#135748 introduced a `Stable` implementation [here](https://github.com/rust-lang/rust/pull/135748/files#diff-60f5e8edf69e04e89ef0c7f576363a91fa141e1db969484cef00063ed39c62e4R235).

Rollup merge of rust-lang#136590 - carolynzech:raw-ptr-kind-internal, r=compiler-errors Implement RustcInternal for RawPtrKind Implement `RustcInternal` for `RawPtrKind`. rust-lang#135748 introduced a `Stable` implementation [here](https://github.com/rust-lang/rust/pull/135748/files#diff-60f5e8edf69e04e89ef0c7f576363a91fa141e1db969484cef00063ed39c62e4R235).

rustbot assigned lcnr Jan 19, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 19, 2025

This comment has been minimized.

Sign in to view

scottmcm reviewed Jan 19, 2025

View reviewed changes

compiler-errors force-pushed the len-2 branch from 89c30c7 to a8e871e Compare January 20, 2025 04:21

RalfJung reviewed Jan 20, 2025

View reviewed changes

compiler-errors force-pushed the len-2 branch from a8e871e to 7176c19 Compare January 20, 2025 19:10

RalfJung reviewed Jan 20, 2025

View reviewed changes

compiler/rustc_const_eval/src/check_consts/check.rs Outdated Show resolved Hide resolved

RalfJung reviewed Jan 20, 2025

View reviewed changes

compiler/rustc_middle/src/mir/syntax.rs Outdated Show resolved Hide resolved

RalfJung reviewed Jan 20, 2025

View reviewed changes

compiler/rustc_middle/src/mir/syntax.rs Outdated Show resolved Hide resolved

compiler-errors force-pushed the len-2 branch 2 times, most recently from ef8afa1 to 2f77014 Compare January 21, 2025 17:42

scottmcm mentioned this pull request Jan 22, 2025

Get rid of mir::Const::from_ty_const #135753

Merged

compiler-errors force-pushed the len-2 branch from 2f77014 to 16f4106 Compare January 23, 2025 18:32

RalfJung reviewed Jan 23, 2025

View reviewed changes

compiler/rustc_const_eval/src/check_consts/check.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

compiler-errors force-pushed the len-2 branch from 16f4106 to 8b487a1 Compare January 23, 2025 18:47

RalfJung approved these changes Jan 23, 2025

View reviewed changes

rustbot assigned oli-obk and unassigned lcnr Jan 27, 2025

compiler-errors force-pushed the len-2 branch from 8b487a1 to 7b9f0cb Compare January 27, 2025 19:37

compiler-errors commented Jan 27, 2025

View reviewed changes

compiler/rustc_borrowck/src/type_check/mod.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

compiler-errors force-pushed the len-2 branch from 7b9f0cb to dcd4784 Compare January 27, 2025 20:11

compiler-errors added 3 commits January 27, 2025 23:32

Type level consts can show up in MIR type checker

1cbb062

Reapply "Auto merge of rust-lang#133734 - scottmcm:lower-indexing-to-…

057313b

…ptrmetadata, r=davidtwco,RalfJung" This reverts commit 122a55b.

Represent the raw pointer for a array length check as a new kind of f…

eeecb56

…ake borrow

compiler-errors force-pushed the len-2 branch from dcd4784 to eeecb56 Compare January 28, 2025 00:01

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 28, 2025

This was referenced Jan 28, 2025

Rollup of 9 pull requests #136184

Closed

Rollup of 8 pull requests #136185

Merged

bors merged commit 21ddd7a into rust-lang:master Jan 28, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Jan 28, 2025

matthiaskrgr mentioned this pull request Jan 30, 2025

ICE: fake borrows are always indirect #136298

Closed

carolynzech mentioned this pull request Feb 5, 2025

Implement RustcInternal for RawPtrKind #136590

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lower index bounds checking to `PtrMetadata`, this time with the right fake borrow semantics 😸 #135748

Lower index bounds checking to `PtrMetadata`, this time with the right fake borrow semantics 😸 #135748

compiler-errors commented Jan 19, 2025

rustbot commented Jan 19, 2025

rustbot commented Jan 19, 2025

This comment has been minimized.

scottmcm Jan 19, 2025 •

edited

Loading

compiler-errors Jan 19, 2025

compiler-errors Jan 20, 2025

scottmcm Jan 20, 2025

scottmcm left a comment

This comment has been minimized.

RalfJung left a comment

bors commented Jan 26, 2025

compiler-errors commented Jan 27, 2025

This comment has been minimized.

oli-obk commented Jan 28, 2025

bors commented Jan 28, 2025

Lower index bounds checking to PtrMetadata, this time with the right fake borrow semantics 😸 #135748

Lower index bounds checking to PtrMetadata, this time with the right fake borrow semantics 😸 #135748

Conversation

compiler-errors commented Jan 19, 2025

rustbot commented Jan 19, 2025

rustbot commented Jan 19, 2025

This comment has been minimized.

scottmcm Jan 19, 2025 • edited Loading

Choose a reason for hiding this comment

compiler-errors Jan 19, 2025

Choose a reason for hiding this comment

compiler-errors Jan 20, 2025

Choose a reason for hiding this comment

scottmcm Jan 20, 2025

Choose a reason for hiding this comment

scottmcm left a comment

Choose a reason for hiding this comment

This comment has been minimized.

RalfJung left a comment

Choose a reason for hiding this comment

bors commented Jan 26, 2025

compiler-errors commented Jan 27, 2025

This comment has been minimized.

oli-obk commented Jan 28, 2025

bors commented Jan 28, 2025

Lower index bounds checking to `PtrMetadata`, this time with the right fake borrow semantics 😸 #135748

Lower index bounds checking to `PtrMetadata`, this time with the right fake borrow semantics 😸 #135748

scottmcm Jan 19, 2025 •

edited

Loading