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

[moxian]

@rustbot label: +A-const-eval +T-compiler +T-lang

[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)