Suboptimal diagnostics when "&function()" in const is rejected due to interior mutability

[camsteffen]

I expected to be able to write

const { &Backtrace::disabled() }

but it yields the error

error[E0492]: constants cannot refer to interior mutable data

This is surprising because:

1. A disabled Backtrace is an empty enum variant without interior mutable data.
2. The error refers to a library implementation detail which is a bit confusing.
3. This seems like something I ought to be able to do.

Although there is an easy workaround which is to use a static.

Rust version: 1.88.0

[bjorn3]

A disabled Backtrace is an empty enum variant without interior mutable data.

What matters is if the type implements Freeze (aka doesn't have interior mutable data) or not. Not whether the specific value doesn't contain interior mutable data.

rust/compiler/rustc_const_eval/src/check_consts/qualifs.rs

Lines 87 to 124 in d612081

	fn in_any_value_of_ty<'tcx>(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool {
	// Avoid selecting for simple cases, such as builtin types.
	if ty.is_trivially_freeze() {
	return false;
	}
	// Avoid selecting for `UnsafeCell` either.
	if ty.ty_adt_def().is_some_and(|adt| adt.is_unsafe_cell()) {
	return true;
	}
	// We do not use `ty.is_freeze` here, because that requires revealing opaque types, which
	// requires borrowck, which in turn will invoke mir_const_qualifs again, causing a cycle error.
	// Instead we invoke an obligation context manually, and provide the opaque type inference settings
	// that allow the trait solver to just error out instead of cycling.
	let freeze_def_id = cx.tcx.require_lang_item(LangItem::Freeze, cx.body.span);
	// FIXME(#132279): Once we've got a typing mode which reveals opaque types using the HIR
	// typeck results without causing query cycles, we should use this here instead of defining
	// opaque types.
	let typing_env = ty::TypingEnv {
	typing_mode: ty::TypingMode::analysis_in_body(
	cx.tcx,
	cx.body.source.def_id().expect_local(),
	),
	param_env: cx.typing_env.param_env,
	};
	let (infcx, param_env) = cx.tcx.infer_ctxt().build_with_typing_env(typing_env);
	let ocx = ObligationCtxt::new(&infcx);
	let obligation = Obligation::new(
	cx.tcx,
	ObligationCause::dummy_with_span(cx.body.span),
	param_env,
	ty::TraitRef::new(cx.tcx, freeze_def_id, [ty::GenericArg::from(ty)]),
	);
	ocx.register_obligation(obligation);
	let errors = ocx.select_all_or_error();
	!errors.is_empty()
	}

If we were to check the actual value, that would actually leak library implementation details as we don't guarantee that Backtrace has the Disabled variant outside of the LazyLock.

The error refers to a library implementation detail which is a bit confusing.

Whether or not a type is Freeze is part of the public api. And Backtrace is documented as being !Freeze: https://doc.rust-lang.org/stable/std/backtrace/struct.Backtrace.html#impl-Freeze-for-Backtrace

[camsteffen]

@bjorn3 thanks for explaining.

A couple ideas then:

1. Make the error mention Freeze
   (Even though it's unstable, I think it would be less confusing to point at the public API. It might be good to keep the part about interior mutability, but add on to the message something like ... (i.e. does not implement Freeze)).
2. Suggest using static.

[bjorn3]

On nightly we are already suggesting static:

error[E0492]: interior mutable shared borrows of temporaries that have their lifetime extended until the end of the program are not allowed
 --> src/main.rs:2:13
  |
2 |     const { &std::backtrace::Backtrace::disabled() };
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this borrow of an interior mutable value refers to such a temporary
  |
  = note: Temporaries in constants and statics can have their lifetime extended until the end of the program
  = note: To avoid accidentally creating global mutable state, such temporaries must be immutable
  = help: If you really want global mutable state, try replacing the temporary by an interior mutable `static` or a `static mut`

[RalfJung]

The error refers to a library implementation detail which is a bit confusing.

The fact that it's in a library is exactly why the specific enum variant used by that function does not matter -- we do not look inside that function to determine whether there is interior mutability in its return value, that would be a major semver hazard.

Make the error mention Freeze

I strongly disagree, it would be a bad idea to mention unstable compiler implementation details.

What matters for library users is that Backtrace is a type with interior mutability, not which trait rustc internally uses to determine that.

This seems like something I ought to be able to do.

You mean implicitly creating global mutable state in general, or in this specific case? I assume the latter, since doing this in general is IMO clearly undesirable. For each specific case, this is something a library author can decide to expose, by having a function like:

pub const fn disabled_ref() -> &'static Backtrace {
  &Backtrace { inner: Inner::Disabled }
}

This closes some doors for the potential future evolution of the library, so it's not something we want to do without the consent of the library author.

if you have a good concrete use-case for this, I'd suggest filing an ACP with the libs team.

[camsteffen]

The error refers to a library implementation detail which is a bit confusing.

The fact that it's in a library is exactly why the specific enum variant used by that function does not matter -- we do not look inside that function to determine whether there is interior mutability in its return value, that would be a major semver hazard.

Yes I understand that now. What I mean is that, when an error says, "you can't do X because Y", I generally expect Y to be a fact that I can observe for myself either by looking at my own code or by looking at the public API of what I'm using. When the error says that Backtrace has interior mutable data, this isn't something that I can observe for myself without looking at the library source and that's what feels off to me.

Make the error mention Freeze

I strongly disagree, it would be a bad idea to mention unstable compiler implementation details.

What matters for library users is that Backtrace is a type with interior mutability, not which trait rustc internally uses to determine that.

Fair enough. The (small) problem though is that we are not offering a way to build a mental model for predicting whether const { &... } will compile. If I know about Freeze, I can look at the docs and know. I believe Freeze (or NoCell) will likely be stabilized eventually and then we can mention it in the error?

The disabled_ref idea sounds nice. I might propose that. In short, my use case was to have a #[cfg(debug_assertions)] backtrace: Backtrace field on a custom error type, and then have a fn backtrace(&self) -> &Backtrace function on that type which would return a disabled backtrace if the field is not cfg-enabled.

[RalfJung]

Fair enough. The (small) problem though is that we are not offering a way to build a mental model for predicting whether const { &... } will compile. If I know about Freeze, I can look at the docs and know. I believe Freeze (or NoCell) will likely be stabilized eventually and then we can mention it in the error?

Yeah it is unfortunate that the only way rustdoc surfaces this is through Freeze. It is also unfortunate that these corner-cases of const checking are pretty much the only way you can even observe whether some type "has interior mutability" (as in, "does not implement Freeze"), so it's something people rarely think about. And indeed if that trait gets stabilized Soon (TM) it should be mentioned... but it'll be tricky to do this right, since we do sometimes look at the value of the borrow operand, not just at its type. But we only do that when you directly borrow an enum variant, like &None or so, not through function calls.

So really this is about giving more precise feedback for why exactly this got rejected. Which is fair, but tricky to implement, and even trickier to implement without extra compile-time cost for when there is no error.

Cc @rust-lang/wg-const-eval

[camsteffen]

...since we do sometimes look at the value of the borrow operand, not just at its type. But we only do that when you directly borrow an enum variant, like &None or so, not through function calls.

Which is exactly what you exploited in your disabled_ref example, yes?

[Jules-Bertholet]

The error refers to a library implementation detail which is a bit confusing.

The fact that it's in a library is exactly why the specific enum variant used by that function does not matter -- we do not look inside that function to determine whether there is interior mutability in its return value, that would be a major semver hazard.

I am not quite convinced. The return value of a function is generally considered part of its public API. If Backtrace::disabled() stopped returning a value representing a disabled backtrace, that would be a breaking change. If i32::abs stopped returning absolute values, that would be a breaking change. Etc…

[camsteffen]

The return value of a function is generally considered part of its public API.

Only the behavior of the returned value is part of the public API. The specific enum variant is not.

[RalfJung]

The exact return value is not part of the public API, only its "meaning". Inner::Disabled could gain a field of type Cell and fn disabled could initialize it. That is entirely semver-compatible, so we don't want to accept code that would get rejected in such a future version of the library.