Don't evaluate const blocks in constant promotion

Author: dianne

compiler/rustc_mir_transform/src/promote_consts.rs

@@ -18,6 +18,7 @@ use rustc_const_eval::check_consts::{ConstCx, qualifs};
 use rustc_data_structures::assert_matches;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
+use rustc_hir::def::DefKind;
 use rustc_index::{IndexSlice, IndexVec};
 use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
 use rustc_middle::mir::*;
@@ -329,6 +330,7 @@ impl<'tcx> Validator<'_, 'tcx> {
                 if let TempState::Defined { location: loc, .. } = self.temps[local]
                     && let Left(statement) =  self.body.stmt_at(loc)
                     && let Some((_, Rvalue::Use(Operand::Constant(c)))) = statement.kind.as_assign()
+                    && self.should_evaluate_for_promotion_checks(c.const_)
                     && let Some(idx) = c.const_.try_eval_target_usize(self.tcx, self.typing_env)
                     // Determine the type of the thing we are indexing.
                     && let ty::Array(_, len) = place_base.ty(self.body, self.tcx).ty.kind()
@@ -484,7 +486,9 @@ impl<'tcx> Validator<'_, 'tcx> {
                             let sz = lhs_ty.primitive_size(self.tcx);
                             // Integer division: the RHS must be a non-zero const.
                             let rhs_val = match rhs {
-                                Operand::Constant(c) => {
+                                Operand::Constant(c)
+                                    if self.should_evaluate_for_promotion_checks(c.const_) =>
+                                {
                                     c.const_.try_eval_scalar_int(self.tcx, self.typing_env)
                                 }
                                 _ => None,
@@ -502,9 +506,14 @@ impl<'tcx> Validator<'_, 'tcx> {
                                         // The RHS is -1 or unknown, so we have to be careful.
                                         // But is the LHS int::MIN?
                                         let lhs_val = match lhs {
-                                            Operand::Constant(c) => c
-                                                .const_
-                                                .try_eval_scalar_int(self.tcx, self.typing_env),
+                                            Operand::Constant(c)
+                                                if self.should_evaluate_for_promotion_checks(
+                                                    c.const_,
+                                                ) =>
+                                            {
+                                                c.const_
+                                                    .try_eval_scalar_int(self.tcx, self.typing_env)
+                                            }
                                             _ => None,
                                         };
                                         let lhs_min = sz.signed_int_min();
@@ -683,6 +692,28 @@ impl<'tcx> Validator<'_, 'tcx> {
         // This passed all checks, so let's accept.
         Ok(())
     }
+
+    /// Can we try to evaluate a given constant at this point in compilation? Attempting to evaluate
+    /// a const block before borrow-checking will result in a query cycle (#150464).
+    fn should_evaluate_for_promotion_checks(&self, constant: Const<'tcx>) -> bool {
+        match constant {
+            // `Const::Ty` is always a `ConstKind::Param` right now and that can never be turned
+            // into a mir value for promotion
+            // FIXME(mgca): do we want uses of type_const to be normalized during promotion?
+            Const::Ty(..) => false,
+            Const::Val(..) => true,
+            // Evaluating a MIR constant requires borrow-checking it. For inline consts, as of
+            // #138499, this means borrow-checking its typeck root. Since borrow-checking the
+            // typeck root requires promoting its constants, trying to evaluate an inline const here
+            // will result in a query cycle. To avoid the cycle, we can't evaluate const blocks yet.
+            // Other kinds of unevaluated's can cause query cycles too when they arise from
+            // self-reference in user code; e.g. evaluating a constant can require evaluating a
+            // const function that uses that constant, again requiring evaluation of the constant.
+            // However, this form of cycle renders both the constant and function unusable in
+            // general, so we don't need to special-case it here.
+            Const::Unevaluated(uc, _) => self.tcx.def_kind(uc.def) != DefKind::InlineConst,
+        }
+    }
 }
 
 fn validate_candidates(

tests/ui/inline-const/dont-eval-const-block-during-promotion.fail.stderr

@@ -0,0 +1,42 @@
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/dont-eval-const-block-during-promotion.rs:48:14
+   |
+LL |         x = &([0][const { 0 }] & 0);
+   |              ^^^^^^^^^^^^^^^^^^^^^^- temporary value is freed at the end of this statement
+   |              |
+   |              creates a temporary value which is freed while still in use
+...
+LL |         (x, y, z);
+   |          - borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
+
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/dont-eval-const-block-during-promotion.rs:50:14
+   |
+LL |         y = &(1 / const { 1 });
+   |              ^^^^^^^^^^^^^^^^^- temporary value is freed at the end of this statement
+   |              |
+   |              creates a temporary value which is freed while still in use
+...
+LL |         (x, y, z);
+   |             - borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
+
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/dont-eval-const-block-during-promotion.rs:52:14
+   |
+LL |         z = &(const { 1 } / -1);
+   |              ^^^^^^^^^^^^^^^^^^- temporary value is freed at the end of this statement
+   |              |
+   |              creates a temporary value which is freed while still in use
+LL |
+LL |         (x, y, z);
+   |                - borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0716`.

tests/ui/inline-const/dont-eval-const-block-during-promotion.rs

@@ -0,0 +1,65 @@
+//! Test for #150464: as of #138499, trying to evaluate const blocks during constant promotion will
+//! result in a query cycle, so we shouldn't do it. Evaluation can happen when trying to promote
+//! integer division and array indexing, where it's necessary for the operation to succeed to be
+//! able to use it in a promoted constant.
+//@ revisions: pass fail
+//@[pass] check-pass
+
+use std::mem::offset_of;
+
+struct Thing(i32);
+
+fn main() {
+    // For a temporary involving array indexing to be promoted, we evaluate the index to make sure
+    // it's in-bounds. As of #150557 we treat inline constants as maybe-out-of-bounds to avoid the
+    // query cycle from evaluating them. That allows this to compile:
+    let x = &([0][const { 0 }] & 0);
+    // Likewise, integer divisors must be nonzero. Avoiding the query cycle allows this to compile:
+    let y = &(1 / const { 1 });
+    // Likewise, signed integer dividends can't be the integer minimum when the divisor is -1.
+    let z = &(const { 1 } / -1);
+    // These temporaries are all lifetime-extended, so they don't need to be promoted for references
+    // to them to be live later in the block. Generally, code with const blocks in these positions
+    // should compile as long as being promoted isn't necessary for borrow-checking to succeed.
+    (x, y, z);
+
+    // A reduced example from real code (#150464): this can't be promoted since the array is a local
+    // variable, but it still resulted in a query cycle because the index was evaluated for the
+    // bounds-check before checking that. By not evaluating the const block, we avoid the cycle.
+    // Since this doesn't rely on promotion, it should borrow-check successfully.
+    let temp = [0u8];
+    let _ = &(temp[const { 0usize }] & 0u8);
+    // #150464 was reported because `offset_of!` started desugaring to a const block in #148151.
+    let _ = &(temp[offset_of!(Thing, 0)] & 0u8);
+
+    // Similarly, at the time #150464 was reported, the index here was evaluated before checking
+    // that the indexed expression is an array. As above, this can't be promoted, but still resulted
+    // in a query cycle. By not evaluating the const block, we avoid the cycle. Since this doesn't
+    // rely on promotion, it should borrow-check successfully.
+    let temp: &[u8] = &[0u8];
+    let _ = &(temp[const { 0usize }] & 0u8);
+
+    // By no longer promoting these temporaries, they're dropped at the ends of their respective
+    // statements, so we can't refer to them thereafter. This code no longer query-cycles, but it
+    // fails to borrow-check instead.
+    #[cfg(fail)]
+    {
+        let (x, y, z);
+        x = &([0][const { 0 }] & 0);
+        //[fail]~^ ERROR: temporary value dropped while borrowed
+        y = &(1 / const { 1 });
+        //[fail]~^ ERROR: temporary value dropped while borrowed
+        z = &(const { 1 } / -1);
+        //[fail]~^ ERROR: temporary value dropped while borrowed
+        (x, y, z);
+    }
+
+    // Sanity check: those temporaries do promote if the const blocks are removed.
+    // If constant promotion is changed so that these are no longer implicitly promoted, the
+    // comments on this test file should be reworded to reflect that.
+    let (x, y, z);
+    x = &([0][0] & 0);
+    y = &(1 / 1);
+    z = &(1 / -1);
+    (x, y, z);
+}