Const qualification

[oli-obk]

cc @eddyb @RalfJung

Let's figure out what exactly we need, because I'm very confused about it.

So, currently const qualification is doing a few things at once:

1. figure out all the feature gates and forbid things not allowed in constants/statics/const_fn at all or without the feature gates
2. figure out what things are promotable by ruling out values containing Drop or UnsafeCell types (None is always ok, even if Some(value) would not be due to the type, even if Option<T> would technically propagate said type.
3. In order to guarantee this for promotion we also check the bodies of const fn and const for Drop and UnsafeCell value creations, even if it is ignored for the constant itself and only checked when the constant is used in a value checked for promotion

Why I want to stop looking at bodies, and instead just check the final value of constants:

• This analysis is imperfect. E.g. (UnsafeCell::new(42), 42).1 is treated as if you end up with an UnsafeCell value
• If we keep looking at the body, we're essentially saying that a const fn's body is not allowed to change, even for changes which would not change the final value for any input, because such a change might subtly lead to the analysis suddenly thinking there's an UnsafeCell in the final value

Why we cannot just look at the final value right now:

• when promoting associated constants inside a generic function we might not have enough information to actually compute the final value. We'd need to wait for monomorphization to tell us whether the value is problematic. This is obviously not something we want. All the analyses should run before monomophization

Solution brainstorm:

1. don't promote calls to const fn if its return type may contain UnsafeCell or Drop. So Option::<String> is not promoted, even if the actual value is None. (not a breaking change, since there are no stable const fn for which this could break any code)
2. Always assume the worst with associated constants (already the case https://play.rust-lang.org/?gist=36546b7a589178413e28ba09f1cd0201&version=stable&mode=debug&edition=2015 ) So we don't promote associated constant uses unless monomorphized.

[oli-obk]

Ok, just had a quick conversation with @eddyb on discord.

We also need to prevent

• &AtomicUsize::new(42) in constants (because using such a constant would possibly only duplicate the reference, not the atomic value)
  • statics might be fine, but needs some thought
  • might be fine in general if we guarantee also duplicating the referenced atomic
• need to prevent (T::ASSOC_CONST, 42).1 inside polymorphic code if ASSOC_CONST is a value with a Drop impl

Basically anything other than checking the body of a constant won't be useful for polymorphic code. So we'd still not be able to remove the qualification as it exists today.

To make the qualification smarter and generally be able to understand conditional code and loops and such we need to implement the qualification in terms of dataflow

[RalfJung]

We rule out UnsafeCell values because the same promoted will be used multiple times when this code is executed several times, and of course that's observable once mutation enters the picture.

We rule out Drop values because... it's not okay to not run the destructor, because the user didn't actually ask for a static?

[RalfJung]

To make the qualification smarter and generally be able to understand conditional code and loops and such we need to implement the qualification in terms of dataflow

This only applies to promotion, right? I'd not do dataflow. I'd just say we never promote once control flow is involved.

From what I understand, we still only look at the type when checking if a use of a const fn can be promoted, and the resulting value for const.

[oli-obk]

so const FOO: usize = if false { 42 } else { 45 }; won't be usable in let x: &'static usize = &FOO; ?

[RalfJung]

Oh are you saying that we have trouble with promotion even when we are just using a const? Dang...

For const however a value-based analysis seems reasonable, does it not? The value should be the only thing of a const that is actually observable.

How to determine drop/interior mutability of a value is another question. We could traverse the value guided by the type (exactly like the sanity check does) and look for it? This has to stop at unions, but we do not promote code that reads from unions so I think we are good.

[oli-obk]

If the constant is an associated constant we're out of luck wrt promotion checking the value. But I think we currently just pessimistically assume drop and not_freeze, so it could be fine.

[RalfJung]

True, the best we can do at that point is fall back to its type (insofar that is known).

But I see no reason, so far, to ever look at the body of a constant (as opposed to its computed value).

[eddyb]

More recently, I came to the conclusion that the code is spaghetti because it tried really hard to avoid recomputing things, so it ends up with one "qualif" that's modified in a linear pass.

If rewritten into several modular analyses, and one const checker (the only part that would actually emit errors), we can probably have a cleaner setup overall.
And we could even mix polymorphic MIR analysis with post-eval value analysis.

E.g. (UnsafeCell::new(42), 42).1 is treated as if you end up with an UnsafeCell value

That shouldn't be true, I don't think. Only unions have problems like these, structs should be fine.

[RalfJung]

That shouldn't be true, I don't think. Only unions have problems like these, structs should be fine.

The MIR const qualifier does type-based reasoning on projections of pairs, but the old HIR one not.

[abonander]

@oli-obk Unions are currently feature-gated in const fns but are stable in other const contexts, was this intended?

union SignedToUnsigned {
    signed: i32,
    unsigned: u32,
}

// allowed on stable
const UNSIGNED: u32 = unsafe { SignedToUnsigned { signed: 1i32 }.unsigned };

// requires `#![feature(const_fn_union)]`
const fn signedToUnsigned(signed: i32) -> u32 {
    unsafe { SignedToUnsigned { signed }.unsigned }
}

[RalfJung]

@abonander yes. It is too late to gate them in const. And they also do less harm there because we can do sanity checks on const to ensure they match their given type, which is not possible for const fn.