const checking: do not do value-based reasoning for interior mutability #121786

RalfJung · 2024-02-29T09:58:45Z

This basically means a nicer error for #121610, see the new test in tests/ui/consts/refs-to-cell-in-final.rs.

Currently we have a mismatch between the dynamic semantics of & (things might be mutable when the pointee type is !Freeze, no matter the pointee value) and const-qualif (doing value-based reasoning). This removes that mismatch. I don't think this can break anything that builds on current nightly; any such code would already later run into the error that the interner complains about the mutable pointer (that's exactly #121610). So a crater run makes little sense.

Overall this peanuts compared to the actual problem with value-based reasoning for interior mutability in promotion (rust-lang/unsafe-code-guidelines#493). I have no idea how to resolve that. But at least now the use of the problematic qualif is down to 1...

The first commit is independent, I just got worried about promotion of unions and was happy to notice that at least there we don't do value-based reasoning.

r? @oli-obk

…n of unions

RalfJung · 2024-02-29T10:21:05Z

Lol, so much for this not being a breaking change. It breaks bootstraping.^^

error[E0492]: constants cannot refer to interior mutable data
  --> /home/r/.cargo/registry/src/index.crates.io-6f17d22bba15001f/litemap-0.7.2/src/store/slice_impl.rs:15:33
   |
15 |     const EMPTY: &'a [(K, V)] = &[];
   |                                 ^^^ this borrow of an interior mutable value may end up in the final value

Why does that not get promoted, though...?

RalfJung · 2024-02-29T10:49:15Z

Surrounding code can be seen here.

This does not get promoted because the type of the slice is not known to be 'static. Interesting, I didn't realize such lifetime constraints factor into promotion decisions...

…s are not interior mutable

RalfJung · 2024-03-02T09:20:20Z

Very strange, this breaks the following code:

const NONE: &'static Option<AtomicUsize> = &None;

This gets promoted, but then there are reborrows in the const itself. But reborrows should be ignored by const-checking...

RalfJung · 2024-03-02T09:36:15Z

Oh wait is const-checking happening before promotion?

RalfJung · 2024-03-02T09:38:42Z

Hm yes that may be the case. In that case I am very surprised that this does not lead to more errors... now I wonder what crater would say.
@bors try

bors · 2024-03-02T09:39:51Z

⌛ Trying commit ac1ed22 with merge ca7d067...

…g, r=<try> const checking: do not do value-based reasoning for interior mutability This basically means a nicer error for rust-lang#121610, see the new test in `tests/ui/consts/refs-to-cell-in-final.rs`. Currently we have a mismatch between the dynamic semantics of `&` (things might be mutable when the pointee type is `!Freeze`, no matter the pointee value) and const-qualif (doing value-based reasoning). This removes that mismatch. I don't think this can break anything that builds on current nightly; any such code would already later run into the error that the interner complains about the mutable pointer (that's exactly rust-lang#121610). So a crater run makes little sense. Overall this peanuts compared to the *actual* problem with value-based reasoning for interior mutability in promotion (rust-lang/unsafe-code-guidelines#493). I have no idea how to resolve that. But at least now the use of the problematic qualif is down to 1... The first commit is independent, I just got worried about promotion of unions and was happy to notice that at least there we don't do value-based reasoning. r? `@oli-obk`

RalfJung · 2024-03-02T09:40:06Z

Ah wait this doesn't entirely remove value-based reasoning from promotion, it only makes that not work inside const any more. Never mind.

It is probably futile to use different checks in promotion and const-checking here...

When it comes to promotion, even rustc itself relies on value-based reasoning:

rust/compiler/rustc_pattern_analysis/src/pat.rs

Lines 229 to 234 in be29cd1

    
           pub(crate) fn ctor(self) -> &'p Constructor<Cx> { 
        
               match self { 
        
                   PatOrWild::Wild => &Wildcard, 
        
                   PatOrWild::Pat(pat) => pat.ctor(), 
        
               } 
        
           }

Wildcard here is of type Constructor<Cx> which involves some associated types of Cx which we do not know to be Freeze at this point.

rustbot · 2024-03-02T09:48:51Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rust-log-analyzer · 2024-03-02T09:59:10Z

The job x86_64-gnu-llvm-16 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

#12 writing image sha256:297e8dfe892dd1c08dbbf1d4b9bc5c25bff8b8b29a8e84fa658ba844013243ef done
#12 naming to docker.io/library/rust-ci done
#12 DONE 10.2s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-16]
##[group]Clock drift check
  local time: Sat Mar  2 09:50:48 UTC 2024
  network time: Sat, 02 Mar 2024 09:50:48 GMT
  network time: Sat, 02 Mar 2024 09:50:48 GMT
##[endgroup]
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-16', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'build.optimized-compiler-builtins', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-16/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
error[E0515]: cannot return value referencing temporary value
   --> compiler/rustc_pattern_analysis/src/pat.rs:229:9
    |
229 | /         match self {
230 | |             PatOrWild::Wild => &Wildcard,
    | |                                 -------- temporary value created here
231 | |             PatOrWild::Pat(pat) => pat.ctor(),
232 | |         }
    | |_________^ returns a value referencing data owned by the current function
For more information about this error, try `rustc --explain E0515`.
error: could not compile `rustc_pattern_analysis` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:07:39

RalfJung · 2024-03-02T10:14:13Z

I don't think there's anything we can do here, unless and until we decide to properly tackle value-based reasoning for interior mutability...

… r=oli-obk Add tests (and a bit of cleanup) for interior mut handling in promotion and const-checking Basically these are the parts of rust-lang#121786 that can be salvaged. r? `@oli-obk`

… r=oli-obk Add tests (and a bit of cleanup) for interior mut handling in promotion and const-checking Basically these are the parts of rust-lang#121786 that can be salvaged. r? ``@oli-obk``

Rollup merge of rust-lang#121893 - RalfJung:const-interior-mut-tests, r=oli-obk Add tests (and a bit of cleanup) for interior mut handling in promotion and const-checking Basically these are the parts of rust-lang#121786 that can be salvaged. r? ``@oli-obk``

Add tests (and a bit of cleanup) for interior mut handling in promotion and const-checking Basically these are the parts of rust-lang/rust#121786 that can be salvaged. r? ``@oli-obk``

RalfJung · 2024-03-20T18:50:33Z

Oh wait is const-checking happening before promotion?

For the record -- yes it does: mir_promoted first calls mir_const_qualif, which does const-checking, and then it runs the promote_consts pass.

RalfJung added 2 commits February 29, 2024 10:14

add comment and test: we do not do value-based reasoning for promotio…

6a95896

…n of unions

const checking: do not do value-based reasoning for interior mutability

3ad70a1

rustbot assigned oli-obk Feb 29, 2024

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 Feb 29, 2024

RalfJung mentioned this pull request Feb 29, 2024

regression: encountered mutable pointer in final value when "outer scope" rule applies in const/static with interior mutability #121610

Closed

This comment has been minimized.

Sign in to view

add an interesting test; make const-check understand that empty array…

ac1ed22

…s are not interior mutable

This comment has been minimized.

Sign in to view

experiment: do not use value-based interior mut reasoning for promotion

c7d99c8

RalfJung mentioned this pull request Mar 2, 2024

Add tests (and a bit of cleanup) for interior mut handling in promotion and const-checking #121893

Merged

RalfJung closed this Mar 2, 2024

RalfJung deleted the interior-mut-value-reasoning branch August 2, 2024 12:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

const checking: do not do value-based reasoning for interior mutability #121786

const checking: do not do value-based reasoning for interior mutability #121786

RalfJung commented Feb 29, 2024 •

edited

Loading

This comment has been minimized.

RalfJung commented Feb 29, 2024 •

edited

Loading

RalfJung commented Feb 29, 2024 •

edited

Loading

This comment has been minimized.

RalfJung commented Mar 2, 2024

RalfJung commented Mar 2, 2024

RalfJung commented Mar 2, 2024

bors commented Mar 2, 2024

RalfJung commented Mar 2, 2024 •

edited

Loading

rustbot commented Mar 2, 2024

rust-log-analyzer commented Mar 2, 2024

RalfJung commented Mar 2, 2024

RalfJung commented Mar 20, 2024

const checking: do not do value-based reasoning for interior mutability #121786

const checking: do not do value-based reasoning for interior mutability #121786

Conversation

RalfJung commented Feb 29, 2024 • edited Loading

This comment has been minimized.

RalfJung commented Feb 29, 2024 • edited Loading

RalfJung commented Feb 29, 2024 • edited Loading

This comment has been minimized.

RalfJung commented Mar 2, 2024

RalfJung commented Mar 2, 2024

RalfJung commented Mar 2, 2024

bors commented Mar 2, 2024

RalfJung commented Mar 2, 2024 • edited Loading

rustbot commented Mar 2, 2024

rust-log-analyzer commented Mar 2, 2024

RalfJung commented Mar 2, 2024

RalfJung commented Mar 20, 2024

RalfJung commented Feb 29, 2024 •

edited

Loading

RalfJung commented Feb 29, 2024 •

edited

Loading

RalfJung commented Feb 29, 2024 •

edited

Loading

RalfJung commented Mar 2, 2024 •

edited

Loading