Rollup merge of rust-lang#65485 - ecstatic-morse:const-validation-mismatch-ugliness, r=eddyb

Suppress ICE when validators disagree on `LiveDrop`s in presence of `&mut`

Resolves rust-lang#65394.

This hack disables the validator mismatch ICE in cases where a `MutBorrow` error has been emitted by both validators, but they don't agree on the number of `LiveDrop` errors.

The new validator is more conservative about whether a value is moved from in the presence of mutable borrows. For example, the new validator will emit a `LiveDrop` error on the following code.

```rust
const _: Vec<i32> = {
    let mut x = Vec::new();
    let px = &mut x as *mut _;
    let y = x;
    unsafe { ptr::write(px, Vec::new()); }
    y
};
```

This code is not UB AFAIK (it passes MIRI at least). The current validator does not emit a `LiveDrop` error for `x` upon exit from the initializer. `x` is not actually dropped, so I think this is correct? A proper fix for this would require a new `MaybeInitializedLocals` dataflow analysis or maybe a relaxation of the existing `IndirectlyMutableLocals` one.

r? @RalfJung

Author: Centril

src/librustc_mir/lib.rs

@@ -14,6 +14,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment!
 #![feature(core_intrinsics)]
 #![feature(const_fn)]
 #![feature(decl_macro)]
+#![feature(drain_filter)]
 #![feature(exhaustive_patterns)]
 #![feature(never_type)]
 #![feature(specialization)]

src/librustc_mir/transform/qualify_consts.rs

@@ -1024,23 +1024,12 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
             new_errors.dedup();
 
             if self.errors != new_errors {
-                error!("old validator: {:?}", self.errors);
-                error!("new validator: {:?}", new_errors);
-
-                // ICE on nightly if the validators do not emit exactly the same errors.
-                // Users can supress this panic with an unstable compiler flag (hopefully after
-                // filing an issue).
-                let opts = &self.tcx.sess.opts;
-                let trigger_ice = opts.unstable_features.is_nightly_build()
-                    && !opts.debugging_opts.suppress_const_validation_back_compat_ice;
-
-                if trigger_ice {
-                    span_bug!(
-                        body.span,
-                        "{}",
-                        VALIDATOR_MISMATCH_ERR,
-                    );
-                }
+                validator_mismatch(
+                    self.tcx,
+                    body,
+                    std::mem::replace(&mut self.errors, vec![]),
+                    new_errors,
+                );
             }
         }
 
@@ -1870,6 +1859,58 @@ fn args_required_const(tcx: TyCtxt<'_>, def_id: DefId) -> Option<FxHashSet<usize
     Some(ret)
 }
 
+fn validator_mismatch(
+    tcx: TyCtxt<'tcx>,
+    body: &Body<'tcx>,
+    mut old_errors: Vec<(Span, String)>,
+    mut new_errors: Vec<(Span, String)>,
+) {
+    error!("old validator: {:?}", old_errors);
+    error!("new validator: {:?}", new_errors);
+
+    // ICE on nightly if the validators do not emit exactly the same errors.
+    // Users can supress this panic with an unstable compiler flag (hopefully after
+    // filing an issue).
+    let opts = &tcx.sess.opts;
+    let strict_validation_enabled = opts.unstable_features.is_nightly_build()
+        && !opts.debugging_opts.suppress_const_validation_back_compat_ice;
+
+    if !strict_validation_enabled {
+        return;
+    }
+
+    // If this difference would cause a regression from the old to the new or vice versa, trigger
+    // the ICE.
+    if old_errors.is_empty() || new_errors.is_empty() {
+        span_bug!(body.span, "{}", VALIDATOR_MISMATCH_ERR);
+    }
+
+    // HACK: Borrows that would allow mutation are forbidden in const contexts, but they cause the
+    // new validator to be more conservative about when a dropped local has been moved out of.
+    //
+    // Supress the mismatch ICE in cases where the validators disagree only on the number of
+    // `LiveDrop` errors and both observe the same sequence of `MutBorrow`s.
+
+    let is_live_drop = |(_, s): &mut (_, String)| s.starts_with("LiveDrop");
+    let is_mut_borrow = |(_, s): &&(_, String)| s.starts_with("MutBorrow");
+
+    let old_live_drops: Vec<_> = old_errors.drain_filter(is_live_drop).collect();
+    let new_live_drops: Vec<_> = new_errors.drain_filter(is_live_drop).collect();
+
+    let only_live_drops_differ = old_live_drops != new_live_drops && old_errors == new_errors;
+
+    let old_mut_borrows = old_errors.iter().filter(is_mut_borrow);
+    let new_mut_borrows = new_errors.iter().filter(is_mut_borrow);
+
+    let at_least_one_mut_borrow = old_mut_borrows.clone().next().is_some();
+
+    if only_live_drops_differ && at_least_one_mut_borrow && old_mut_borrows.eq(new_mut_borrows) {
+        return;
+    }
+
+    span_bug!(body.span, "{}", VALIDATOR_MISMATCH_ERR);
+}
+
 const VALIDATOR_MISMATCH_ERR: &str =
     r"Disagreement between legacy and dataflow-based const validators.
     After filing an issue, use `-Zsuppress-const-validation-back-compat-ice` to compile your code.";

src/test/ui/consts/const-eval/issue-65394.rs

@@ -0,0 +1,13 @@
+// Test for absence of validation mismatch ICE in #65394
+
+#![feature(rustc_attrs)]
+
+#[rustc_mir(borrowck_graphviz_postflow="hello.dot")]
+const _: Vec<i32> = {
+    let mut x = Vec::<i32>::new();
+    let r = &mut x; //~ ERROR references in constants may only refer to immutable values
+    let y = x;
+    y
+};
+
+fn main() {}

src/test/ui/consts/const-eval/issue-65394.stderr

@@ -0,0 +1,11 @@
+error[E0017]: references in constants may only refer to immutable values
+  --> $DIR/issue-65394.rs:8:13
+   |
+LL |     let r = &mut x;
+   |             ^^^^^^ constants require immutable values
+
+[ERROR rustc_mir::transform::qualify_consts] old validator: [($DIR/issue-65394.rs:8:13: 8:19, "MutBorrow(Mut { allow_two_phase_borrow: false })")]
+[ERROR rustc_mir::transform::qualify_consts] new validator: [($DIR/issue-65394.rs:8:13: 8:19, "MutBorrow(Mut { allow_two_phase_borrow: false })"), ($DIR/issue-65394.rs:7:9: 7:14, "LiveDrop")]
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0017`.