cleanup promotion const_kind checks

in particular allow a few more promotions for consistency when they were already allowed in other contexts

Author: RalfJung

compiler/rustc_mir/src/transform/promote_consts.rs

@@ -297,6 +297,17 @@ impl std::ops::Deref for Validator<'a, 'tcx> {
 struct Unpromotable;
 
 impl<'tcx> Validator<'_, 'tcx> {
+    //! Determines if this code could be executed at runtime and thus is subject to codegen.
+    //! That means even unused constants need to be evaluated.
+    //!
+    //! `const_kind` should not be used in this file other than through this method!
+    fn maybe_runtime(&self) -> bool {
+        match self.const_kind {
+            None | Some(hir::ConstContext::ConstFn) => true,
+            Some(hir::ConstContext::Static(_) | hir::ConstContext::Const) => false,
+        }
+    }
+
     fn validate_candidate(&self, candidate: Candidate) -> Result<(), Unpromotable> {
         match candidate {
             Candidate::Ref(loc) => {
@@ -365,10 +376,8 @@ impl<'tcx> Validator<'_, 'tcx> {
                             // mutably without consequences. However, only &mut []
                             // is allowed right now, and only in functions.
                             if let ty::Array(_, len) = ty.kind() {
-                                // FIXME(eddyb) the `self.is_non_const_fn` condition
-                                // seems unnecessary, given that this is merely a ZST.
                                 match len.try_eval_usize(self.tcx, self.param_env) {
-                                    Some(0) if self.const_kind.is_none() => {}
+                                    Some(0) => {}
                                     _ => return Err(Unpromotable),
                                 }
                             } else {
@@ -495,9 +504,10 @@ impl<'tcx> Validator<'_, 'tcx> {
         match place {
             PlaceRef { local, projection: [] } => self.validate_local(local),
             PlaceRef { local, projection: [proj_base @ .., elem] } => {
+                // Validate topmost projection, then recurse.
                 match *elem {
                     ProjectionElem::Deref => {
-                        let mut not_promotable = true;
+                        let mut promotable = false;
                         // This is a special treatment for cases like *&STATIC where STATIC is a
                         // global static variable.
                         // This pattern is generated only when global static variables are directly
@@ -512,6 +522,9 @@ impl<'tcx> Validator<'_, 'tcx> {
                             }) = def_stmt
                             {
                                 if let Some(did) = c.check_static_ptr(self.tcx) {
+                                    // Evaluating a promoted may not read statics except if it got
+                                    // promoted from a static (this is a CTFE check). So we
+                                    // can only promoted static accesses inside statics.
                                     if let Some(hir::ConstContext::Static(..)) = self.const_kind {
                                         // The `is_empty` predicate is introduced to exclude the case
                                         // where the projection operations are [ .field, * ].
@@ -524,13 +537,13 @@ impl<'tcx> Validator<'_, 'tcx> {
                                         if proj_base.is_empty()
                                             && !self.tcx.is_thread_local_static(did)
                                         {
-                                            not_promotable = false;
+                                            promotable = true;
                                         }
                                     }
                                 }
                             }
                         }
-                        if not_promotable {
+                        if !promotable {
                             return Err(Unpromotable);
                         }
                     }
@@ -545,7 +558,7 @@ impl<'tcx> Validator<'_, 'tcx> {
                     }
 
                     ProjectionElem::Field(..) => {
-                        if self.const_kind.is_none() {
+                        if self.maybe_runtime() {
                             let base_ty =
                                 Place::ty_from(place.local, proj_base, self.body, self.tcx).ty;
                             if let Some(def) = base_ty.ty_adt_def() {
@@ -571,13 +584,6 @@ impl<'tcx> Validator<'_, 'tcx> {
             // `validate_rvalue` upon access.
             Operand::Constant(c) => {
                 if let Some(def_id) = c.check_static_ptr(self.tcx) {
-                    // Only allow statics (not consts) to refer to other statics.
-                    // FIXME(eddyb) does this matter at all for promotion?
-                    let is_static = matches!(self.const_kind, Some(hir::ConstContext::Static(_)));
-                    if !is_static {
-                        return Err(Unpromotable);
-                    }
-
                     let is_thread_local = self.tcx.is_thread_local_static(def_id);
                     if is_thread_local {
                         return Err(Unpromotable);
@@ -591,20 +597,20 @@ impl<'tcx> Validator<'_, 'tcx> {
 
     fn validate_rvalue(&self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> {
         match *rvalue {
-            Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) if self.const_kind.is_none() => {
+            Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) if self.maybe_runtime() => {
                 let operand_ty = operand.ty(self.body, self.tcx);
                 let cast_in = CastTy::from_ty(operand_ty).expect("bad input type for cast");
                 let cast_out = CastTy::from_ty(cast_ty).expect("bad output type for cast");
                 match (cast_in, cast_out) {
                     (CastTy::Ptr(_) | CastTy::FnPtr, CastTy::Int(_)) => {
-                        // in normal functions, mark such casts as not promotable
+                        // ptr-to-int casts are not promotable
                         return Err(Unpromotable);
                     }
                     _ => {}
                 }
             }
 
-            Rvalue::BinaryOp(op, ref lhs, _) if self.const_kind.is_none() => {
+            Rvalue::BinaryOp(op, ref lhs, _) if self.maybe_runtime() => {
                 if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.body, self.tcx).kind() {
                     assert!(
                         op == BinOp::Eq
@@ -623,6 +629,7 @@ impl<'tcx> Validator<'_, 'tcx> {
 
             Rvalue::NullaryOp(NullOp::Box, _) => return Err(Unpromotable),
 
+            // FIXME(RalfJung): the rest is *implicitly considered promotable*... that seems dangerous.
             _ => {}
         }
 
@@ -644,8 +651,8 @@ impl<'tcx> Validator<'_, 'tcx> {
             }
 
             Rvalue::AddressOf(_, place) => {
-                // Raw reborrows can come from reference to pointer coercions,
-                // so are allowed.
+                // We accept `&raw *`, i.e., raw reborrows -- creating a raw pointer is
+                // no problem, only using it is.
                 if let [proj_base @ .., ProjectionElem::Deref] = place.projection.as_ref() {
                     let base_ty = Place::ty_from(place.local, proj_base, self.body, self.tcx).ty;
                     if let ty::Ref(..) = base_ty.kind() {
@@ -666,10 +673,8 @@ impl<'tcx> Validator<'_, 'tcx> {
                     // mutably without consequences. However, only &mut []
                     // is allowed right now, and only in functions.
                     if let ty::Array(_, len) = ty.kind() {
-                        // FIXME(eddyb): We only return `Unpromotable` for `&mut []` inside a
-                        // const context which seems unnecessary given that this is merely a ZST.
                         match len.try_eval_usize(self.tcx, self.param_env) {
-                            Some(0) if self.const_kind.is_none() => {}
+                            Some(0) => {}
                             _ => return Err(Unpromotable),
                         }
                     } else {
@@ -734,14 +739,7 @@ impl<'tcx> Validator<'_, 'tcx> {
     ) -> Result<(), Unpromotable> {
         let fn_ty = callee.ty(self.body, self.tcx);
 
-        // `const` and `static` use the explicit rules for promotion regardless of the `Candidate`,
-        // meaning calls to `const fn` can be promoted.
-        let context_uses_explicit_promotion_rules = matches!(
-            self.const_kind,
-            Some(hir::ConstContext::Static(_) | hir::ConstContext::Const)
-        );
-
-        if !self.explicit && !context_uses_explicit_promotion_rules {
+        if !self.explicit && self.maybe_runtime() {
             if let ty::FnDef(def_id, _) = *fn_ty.kind() {
                 // Never promote runtime `const fn` calls of
                 // functions without `#[rustc_promotable]`.

src/test/ui/consts/promote-no-mut.rs

@@ -0,0 +1,30 @@
+// ignore-tidy-linelength
+// Test various things that we do not want to promote.
+#![allow(unconditional_panic, const_err)]
+#![feature(const_fn, const_fn_union)]
+
+// We do not promote mutable references.
+static mut TEST1: Option<&mut [i32]> = Some(&mut [1, 2, 3]); //~ ERROR temporary value dropped while borrowed
+
+static mut TEST2: &'static mut [i32] = {
+    let x = &mut [1,2,3]; //~ ERROR temporary value dropped while borrowed
+    x
+};
+
+// We do not promote fn calls in `fn`, including `const fn`.
+pub const fn promote_cal(b: bool) -> i32 {
+    const fn foo() { [()][42] }
+
+    if b {
+        let _x: &'static () = &foo(); //~ ERROR temporary value dropped while borrowed
+    }
+    13
+}
+
+// We do not promote union field accesses in `fn.
+union U { x: i32, y: i32 }
+pub const fn promote_union() {
+    let _x: &'static i32 = &unsafe { U { x: 0 }.x }; //~ ERROR temporary value dropped while borrowed
+}
+
+fn main() {}

src/test/ui/consts/promote-not.rs

@@ -1,5 +1,5 @@
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-no-mut.rs:3:50
+  --> $DIR/promote-not.rs:7:50
    |
 LL | static mut TEST1: Option<&mut [i32]> = Some(&mut [1, 2, 3]);
    |                                        ----------^^^^^^^^^-
@@ -9,7 +9,7 @@ LL | static mut TEST1: Option<&mut [i32]> = Some(&mut [1, 2, 3]);
    |                                        using this value as a static requires that borrow lasts for `'static`
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-no-mut.rs:6:18
+  --> $DIR/promote-not.rs:10:18
    |
 LL |     let x = &mut [1,2,3];
    |                  ^^^^^^^ creates a temporary which is freed while still in use
@@ -18,6 +18,26 @@ LL |     x
 LL | };
    | - temporary value is freed at the end of this statement
 
-error: aborting due to 2 previous errors
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/promote-not.rs:19:32
+   |
+LL |         let _x: &'static () = &foo();
+   |                 -----------    ^^^^^ creates a temporary which is freed while still in use
+   |                 |
+   |                 type annotation requires that borrow lasts for `'static`
+LL |     }
+   |     - temporary value is freed at the end of this statement
+
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/promote-not.rs:27:29
+   |
+LL |     let _x: &'static i32 = &unsafe { U { x: 0 }.x };
+   |             ------------    ^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use
+   |             |
+   |             type annotation requires that borrow lasts for `'static`
+LL | }
+   | - temporary value is freed at the end of this statement
+
+error: aborting due to 4 previous errors
 
 For more information about this error, try `rustc --explain E0716`.

src/test/ui/consts/promote-no-mut.stderr renamed to src/test/ui/consts/promote-not.stderr

@@ -1,4 +1,4 @@
-// run-pass
+// check-pass
 
 // compile-flags: -O
 

src/test/ui/consts/promotion.rs

@@ -1,4 +1,4 @@
-// check-pass
+// run-pass
 
 // Use of global static variables in literal values should be allowed for
 // promotion.