Confusing error when a const contains a shared ref to interior mutable data

[RalfJung]

I tried this code:

static __CALLSITE: tracing::callsite::DefaultCallsite = {
    const META: tracing::Metadata<'static> = {
        tracing::Metadata::new(
            "my span",
            "tracing_test::a",
            tracing::Level::ERROR,
            Some("src/a.rs"),
            Some(35u32),
            Some("tracing_test::a"),
            tracing::field::FieldSet::new(
                &["message"],
                tracing::callsite::Identifier(&__CALLSITE),
            ),
            tracing::metadata::Kind::SPAN,
        )
    };
    tracing::callsite::DefaultCallsite::new(&META)
};

I expected to see this happen: Either it fails to compile due to a type error or it compiles successfully.

Instead, this happened: "it is undefined behavior to use this value"

Here's a self-contained reproducer:

use std::sync::atomic::*;

static FOO: AtomicU32 = AtomicU32::new(0);
const C: &'static AtomicU32 = &FOO;

yields

error[E0080]: it is undefined behavior to use this value
 --> src/lib.rs:4:1
  |
4 | const C: &'static AtomicU32 = &FOO;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered reference to mutable memory in `const`
  |
  = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
  = note: the raw bytes of the constant (size: 8, align: 8) {
              ╾───────alloc1────────╼                         │ ╾──────╼
          }

I lost context on "constants referencing statics" so I am not sure right now whether this is just a poorly worded error message, or whether there's some static check that should have caught this earlier but for some reason did not.

Cc @rust-lang/wg-const-eval

Meta

Latest nightly

[oli-obk]

We may just want to support this? The static item clearly ensures there is no duplication or deduplication, so using the const to refer to the mutable static should be fine ™

[RalfJung]

One problem with this is patterns: we must not read from mutable state when turning a const pattern into a valtree. So if we allow constants that reference mutable state (rather than just having a raw ptr to it), we end up having consts that look like normal consts but that cause errors when they are used as patterns.

Pattern construction happens during MIR building so these would be pre-monomorphization errors, so maybe that's fine. But avoiding those is why I kept all those strict checks in place.

[RalfJung]

So based on the above I'm pretty sure now that having this error was deliberate, and the final-value validity check is the only place that can really do this (since we want to also forbid &*&raw const STATIC, and we probably want to permit transient references to statics that do not leave the constant).

So the question is between either making the error message more accurate (this isn't UB, it's something we are deliberately forbidding on consts), or just allowing it and accepting that there are some consts that one cannot use as patterns.

@rust-lang/lang any opinion on that?

The problematic examples look something like:

static mut S: i32 = 0;
const C: &'static i32 = unsafe { &S };

unsafe { S = 1; }
// What exactly does this compare with?
match foo {
  C => ...
  _ => ...
}

This example is obviously blatantly unsound, but if we had a type that has interior mutability while also being PartialEq + StructuralPartialEq, one could construct something much more plausible. So I definitely want us to reject code like the above. We have two options for that:

• Complain at the definition of C that this constant is in some sense invalid (status quo)
• Complain at the use of C as a pattern that this constant has a value that makes it invalid to use as a pattern (possible alternative that would then let us accept the example in the OP)

[lcnr]

Is my following understanding of this issue correct?

• the core question is how should we handle constants which reference mutable (global) memory
• right now we forbid the evaluation of the constant from accessing or writing to it, but do allow references to mutable memory playground, and separately check for references to mutable memory in the final value playground
• allowing references to mutable memory in the final value of constants is not unsound by itself, however using these constants in patterns is questionable
  • they should not participate in exhaustiveness checking as they depend on mutable reference
  • having some constants in patterns which do not participate in exhaustiveness feels really weird in surprising
  • could we make mutating the mutable memory referenced by constants in patterns be UB, but allow the constant to participate in exhaustiveness checking? This also seems weird and likely undesirable though

So: assuming that constants referencing mutable memory are totally fine as long as they are not used in patterns, should we:

• keep this error when checking the final value of the constant, but improve the error message to accurately describe the reasoning behind this, or
• allow constants which reference mutable memory, but error if they are used in patterns

[RalfJung]

allowing references to mutable memory in the final value of constants is not unsound by itself, however using these constants in patterns is questionable

Not just questionable, also fundamentally incompatible with the way we compile constants in patterns, which is by turning them into a valtree and turning that into a normal pattern: if you match on an &i32 constant, we'll figure out the value for that at compile-time and hard-code it in the MIR. So we just fundamentally can't compile patterns that involve pointers to mutable data.

But other than that, yes, that sounds exactly correct. :)

[traviscross]

We talked about this in a short-staffed lang call. Both @joshtriplett and I were in favor of allowing these constants and then disallowing them in patterns. Josh presented an interesting use case about how one might reasonably want to use constants like this as a better replacement for global variables when translating code from C.

We'd be interested in seeing a PR to do this nominated for lang so we can propose FCP on it.

[RalfJung]

We'd be interested in seeing a PR to do this nominated for lang so we can propose FCP on it.

PR is up: #140942