Tracking issue for RFC 2342, "Allow `if` and `match` in constants"

[Centril]

This is a tracking issue for the RFC "Allow if and match in constants" (rust-lang/rfcs#2342).

Please redirect constification of specific functions or issues you want to report to fresh issues and label them appropriately with F-const_if_match so that this issues doesn't get flooded with ephemeral comments obscuring important developments.

Steps:

• [x] Implement the RFC
• [ ] Adjust documentation (see instructions on forge)
• [x] Stabilization PR (see instructions on forge): https://github.com/rust-lang/rust/pull/72437
• [x] let bindings in constants that use && and || short circuiting operations. These are treated as & and | inside const and static items right now.

Unresolved questions:

None

[oli-obk]

1. add a feature gate for it
2. switch and switchInt terminators in https://github.com/rust-lang/rust/blob/master/src/librustc_mir/transform/qualify_consts.rs#L347 need to have custom code in case the feature gate is active
3. instead of having a single current basic block (https://github.com/rust-lang/rust/blob/master/src/librustc_mir/transform/qualify_consts.rs#L328) this needs to be some container that has a list of basic blocks it still has to process.

[eddyb]

@oli-obk It's a bit trickier because the complex control-flow means dataflow analysis needs to be employed. I need to get back to @alexreg and figure out how to integrate their changes.

[alexreg]

@eddyb A good starting point would probably be to take my const-qualif branch (minus the top commit), rebase it over master (not going to be fun), and then add data annotation stuff, right?

[mark-i-m]

Any news on this?

[alexreg]

@mark-i-m Alas no. I think @eddyb has been very busy indeed, because I've not even been able to ping him on IRC for the last few weeks hah. Sadly my const-qualif branch doesn't even compile since I last rebased it over master. (I don't believe I've pushed yet though.)

thread 'main' panicked at 'assertion failed: position <= slice.len()', libserialize/leb128.rs:97:1
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: Could not compile `rustc_llvm`.

Caused by:
  process didn't exit successfully: `/Users/alex/Software/rust/build/bootstrap/debug/rustc --crate-name build_script_build librustc_llvm/build.rs --error-format json --crate-type bin --emit=dep-info,link -C opt-level=2 -C metadata=74f2a810ad96be1d -C extra-filename=-74f2a810ad96be1d --out-dir /Users/alex/Software/rust/build/x86_64-apple-darwin/stage1-rustc/release/build/rustc_llvm-74f2a810ad96be1d -L dependency=/Users/alex/Software/rust/build/x86_64-apple-darwin/stage1-rustc/release/deps --extern build_helper=/Users/alex/Software/rust/build/x86_64-apple-darwin/stage1-rustc/release/deps/libbuild_helper-89aaac40d3077cd7.rlib --extern cc=/Users/alex/Software/rust/build/x86_64-apple-darwin/stage1-rustc/release/deps/libcc-ead7d4af4a69e776.rlib` (exit code: 101)
warning: build failed, waiting for other jobs to finish...
error: build failed
command did not execute successfully: "/Users/alex/Software/rust/build/x86_64-apple-darwin/stage0/bin/cargo" "build" "--target" "x86_64-apple-darwin" "-j" "8" "--release" "--manifest-path" "/Users/alex/Software/rust/src/librustc_trans/Cargo.toml" "--features" " jemalloc" "--message-format" "json"
expected success, got: exit code: 101
thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1085:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
failed to run: /Users/alex/Software/rust/build/bootstrap/debug/bootstrap -i build

[alexreg]

Okay, funnily enough, I rebased again just today and it seems to be building all fine now! Looks like there was a regression, and it just got fixed. All over to @eddyb now.

[eddyb]

@alexreg Sorry, I've switched to a local sleep schedule and I see you've pinged me when I wake up but then you're offline all day when I'm awake (ugh timezones). Should I just make a PR out of your branch? I forgot what we were supposed to do with it?

[alexreg]

@eddyb That's alright heh. You must be off to bed early, since I'm usually on from 8:00PM GMT, but it's all good! :-)

[eddyb]

I'm really sorry, took me a while to realize that the series of patches in question requires removing Qualif::STATIC{,_REF}, i.e. the errors about accessing statics at compile-time. OTOH, this is already broken in terms of const fns and access to statics:

#![feature(const_fn)]
const fn read<T: Copy>(x: &T) -> T { *x }
static FOO: u32 = read(&BAR);
static BAR: u32 = 5;
fn main() {
    println!("{}", FOO);
}

This is not detected statically, instead miri complains that "dangling pointer was dereferenced" (which should really say something about statics instead of "dangling pointer").

So I think reading statics at compile-time should be fine, but some people want const fn to be "pure" (i.e. "referentially transparent" or thereabouts) at runtime, which would mean that a const fn reading from behind a reference it got as an argument is fine, but a const fn should never be able to obtain a reference to a static out of thin air (including from consts).

I think then we can keep statically denying mentioning statics (even if only to take their reference) in consts, const fn, and other constant contexts (including promoteds). But we still have to remove the STATIC_REF hack which allows statics to take the reference of other statics but (poorly tries and fails to) deny reading from behind those references.

Do we need an RFC for this?

[alexreg]

Sounds fair w.r.t. reading from statics. Doubt it needs an RFC, maybe just a crater run, but then I’m probably not the best one to say.

[eddyb]

Note that we wouldn't be restricting anything, we'd be relaxing a restriction that's already broken.

[alexreg]

Oh, I misread. So const evaluation would still be sound, just not referentially transparent?

[eddyb]

The last paragraph describes a referentially transparent approach (but we lose that property if we start allowing mentioning statics in consts and const fns). I don't think soundness was really under discussion.

[alexreg]

Well, "dangling pointer" sure sounds like a soundness issue, but I'll trust you on this!

[eddyb]

"dangling pointer" is a bad error message, that's just miri forbidding reading from statics. The only constant contexts that can even refer to statics are other statics, so we could "just" allow those reads, since all of that code always runs once, at compile-time.

(from IRC) To summarize, referentially transparent const fn could only ever reach frozen allocations, without going through arguments, which means const needs the same restriction, and non-frozen allocations can only come from statics.

[Centril]

I do like preserving referential transparency so @eddyb's idea sounds fantastic!

[alexreg]

Yeah I’m pro making const fns pure as well.

[eddyb]

Please note that certain seemingly harmless plans could ruin referential transparency, e.g.:

let x = 0;
let non_deterministic = &x as *const _ as usize;
if non_deterministic.count_ones() % 2 == 0 {
    // do one thing
} else {
    // do a completely different thing
}

This would fail with a miri error at compile-time, but would be non-deterministic at runtime (because we can't mark that memory address as "abstract" like miri can).

EDIT: @Centril had the idea of making certain raw pointer operations (such as comparisons and casts to integers) unsafe within const fn (which we can do up until we stabilize const fn), and state that they can only be used in ways that miri would allow at compile-time. For example, subtracting two pointers into the same local should be fine (you get a relative distance that only depends on the type layout, array indices, etc.), but formatting the address of a reference (via {:p}) is an incorrect use and therefore fmt::Pointer::fmt can't be marked const fn. Also none of the Ord / Eq trait impls for raw pointers can be marked as const (whenever we get the ability to annotate them as such), because they're safe but the operation is unsafe in const fn.

[alexreg]

Depends what you mean by "harmless"... I can certainly see reason we'd want to ban such non-deterministic behaviour.

[lachlansneff]

It would be fantastic if work were continued on this.

[alexreg]

@lachlansneff It's moving... not as quickly as we'd like, but work is being done. At the moment we're waiting on https://github.com/rust-lang/rust/pull/51110 as a blocker.

[lachlansneff]

@alexreg Ah, thank you. It would be very useful to be able to mark a match or if as const even when not in a const fn.

[programmerjake]

any status updates now that #51110 is merged?

[alexreg]

@programmerjake I'm waiting for some feedback from @eddyb on https://github.com/rust-lang/rust/pull/52518 before it can get merged (hopefully very soon). He's been very busy lately (always in high demand), but he's gotten back to reviews and whatnot in the past few days, so I'm hopeful. After that, it will need some work by him personally, I suspect, since adding proper dataflow analysis is a complicated affair. We'll see though.

[RalfJung]

Somewhere to the TODO lists in the first post(s), it should be added to remove the current horrible hack that translates && and || to & and | inside constants.

[alexreg]

@RalfJung Wasn't that part of the old const eval, that's complete gone now that MIRI CTFE is in place?

[RalfJung]

AFAIK we do that translation somewhere in HIR lowering, because we have code in const_qualify that rejects SwitchInt terminators which would otherwise be generated by ||/&&.

Also,another point: @oli-obk said somewhere (but I cannot find where) that conditionals are somehow more complicated than one would naively think... was that "just" about the analysis for drop/interior mutability?

[oli-obk]

was that "just" about the analysis for drop/interior mutability?

I'm currently trying to clear that up. Will get back to you when I have all the information

[mark-i-m]

What's the status of this? Is this in need of manpower or is it blocked on solving some problem?

[alexreg]

@mark-i-m It's blocked on implementing proper dataflow analysis for const qualification. @eddyb is the most knowledgeable in this area, and he had previously done some work on this. (So had I, but that sort of stagnated...) If @eddyb still doesn't have time, perhaps @oli-obk or @RalfJung could tackle this at some point soon. :-)

[eddyb]

58403 is a small step towards dataflow-based qualification.

[jyn514]

@eddyb you mentioned preserving referential transparency in const fn, which I think is a good idea. What if you prevented using pointers in const fn? So your previous code sample would no longer compile:

let x = 0;
// compile time error: cannot cast reference to pointer in `const fun`
let non_deterministic = &x as *const _ as usize;
if non_deterministic.count_ones() % 2 == 0 {
    // do one thing
} else {
    // do a completely different thing
}

References would still be allowed but you wouldn't be allowed to introspect them:

let x = 0;
let p = &x;
if *p != 0 {  // this is fine
    // do one thing
} else {
    // do a completely different thing
}

Let me know if I'm completely off base, I just thought this would be a good way to make this deterministic.

[oli-obk]

@jyn514 that is already covered by making as usize casts unstable (https://github.com/rust-lang/rust/issues/51910), but users can also compare raw pointers (https://github.com/rust-lang/rust/issues/53020) which is just as bad and thus also unstable. We can handle these independently of control flow.

[mark-i-m]

Any new on this?

[oli-obk]

There is some discussion on https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/dataflow-based.20const.20qualification.20MVP

[est31]

@oli-obk your link doesn't work. What does it say?

[alexreg]

It works for me... you've got to sign into Zulip though.

[est31]

@alexreg hmm yeah I presume it was about the dataflow based const qualification work. @alexreg do you know why it's needed for if and match in constants?

[oli-obk]

if we don't have a dataflow based version we either accidentally allow &Cell<T> inside constants or accidentally forbid None::<&Cell<T>> (which works on stable. It's essentially impossible to implement properly without dataflow (or any implementation will be a bad broken ad-hoc version of dataflow)

[alexreg]

@est31 Well, @oli-obk understands this much better than me, but from a high-level basically anything involving branching is going to predicate dataflow analysis unless you want a bunch of edge cases. Anyway, it seems like this person on Zulip is trying to work on it, and if not I know oli-obk and eddyb have intentions to, maybe this month or next (from when I last spoke to them about it), though I can't/won't make promises on their behalf.

[ecstatic-morse]

@alexreg @mark-i-m @est31 @oli-obk I should be able to publish my WIP implementation of dataflow-based const qualification sometime this week. There are a lot of compatibility hazards here, so it may take a while to actually merge it.

[alexreg]

Super; look forward to it.

[jhpratt]

(copying from #57563 per request)

Would it be possible to special-case bool && bool, bool || bool, etc.? They can currently be performed in a const fn, but doing so requires bitwise operators, which is sometimes unwanted.

[RalfJung]

They are already special-cased in const and static items -- by translating them to bitwise operations. But that special-casing is a huge hack and it is very hard to make sure that this is actually correct. As you said, it is also sometimes unwanted. So we'd rather not do this more often.

Doing things right will take a bit, but it'll happen. If we pile on too many hacks in the mean time, we might pain ourselves into a corner that we cannot get out of (if some of those hacks end up interacting in wrong ways, thus accidentally stabilizing behavior we do not want).

[ecstatic-morse]

Now that #64470 and #63812 have been merged, all the tools required for this exist in the compiler. I still need to make some changes to the query system around const qualification to make sure that it isn't needlessly inefficient with this feature enabled. We are making progress here, and I believe an experimental implementation of this will be available on nightly in weeks, not months (famous last words :smile:).

[alexreg]

@ecstatic-morse Great to hear! Thanks for your concerted efforts to get this done; I personally have been keen on this feature for a while now.

Would love to see heap allocation support for CTFE after this is done. I don't know if you or anyone else is interested in working on this, but if not perhaps I could help.

[ecstatic-morse]

@alexreg Thanks!

Discussion about heap allocation at compile time is over at rust-rfcs/const-eval#20. AFAIK, the most recent developments were around a ConstSafe/ConstRefSafe paradigm for determining what can be observed directly/behind a reference in the final value of a const. I think there's more design work needed though.

[ecstatic-morse]

For those following along, #65949 (which itself depends on a few smaller PRs) is the next blocker for this. While it may seem only tangentially related, the fact that const-checking/qualification was so tightly coupled with promotion was part of the reason this feature was blocked for so long. I plan on opening a subsequent PR that will remove the old const-checker entirely (currently we run both checkers in parallel). This will avoid the inefficiencies I mentioned earlier.

After both of the aforementioned PRs are merged, if and match in constants will be a few diagnostics improvements and a feature flag away! Oh, and also tests, so many tests...

[jyn514]

If you need tests, I'm not sure how to get started but I am more than willing to contribute! Just let me know where the tests should go/what they should look like/what branch I should base code off of :)

[ecstatic-morse]

Next PR to watch is #66385. This removes the old const qualification logic (which could not handle branching) completely in favor of the new dataflow-based version.

@jyn514 That would be great! I'll ping you when I start drafting the implementation. It would also be very helpful for people to try to violate const safety (especially the HasMutInterior part) once if and match are available on nightly.