Promoteds also need param envs.

This also allows us to use the `const_eval` query again without causing cycles

Author: oli-obk

src/librustc/mir/interpret/queries.rs

@@ -42,7 +42,7 @@ impl<'tcx> TyCtxt<'tcx> {
         let instance = ty::Instance::resolve(self, param_env, def_id, substs);
         if let Some(instance) = instance {
             if let Some(promoted) = promoted {
-                self.const_eval_promoted(instance, promoted)
+                self.const_eval_promoted(param_env, instance, promoted)
             } else {
                 self.const_eval_instance(param_env, instance, span)
             }
@@ -68,11 +68,11 @@ impl<'tcx> TyCtxt<'tcx> {
     /// Evaluate a promoted constant.
     pub fn const_eval_promoted(
         self,
+        param_env: ty::ParamEnv<'tcx>,
         instance: ty::Instance<'tcx>,
         promoted: mir::Promoted,
     ) -> ConstEvalResult<'tcx> {
         let cid = GlobalId { instance, promoted: Some(promoted) };
-        let param_env = ty::ParamEnv::reveal_all();
         self.const_eval_validated(param_env.and(cid))
     }
 }

src/librustc_mir/interpret/eval_context.rs

@@ -757,13 +757,22 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         &self,
         gid: GlobalId<'tcx>,
     ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
-        let val = if self.tcx.is_static(gid.instance.def_id()) {
-            self.tcx.const_eval_poly(gid.instance.def_id())?
-        } else if let Some(promoted) = gid.promoted {
-            self.tcx.const_eval_promoted(gid.instance, promoted)?
+        // For statics we pick `ParamEnv::reveal_all`, because statics don't have generics
+        // and thus don't care about the parameter environment. While we could just use
+        // `self.param_env`, that would mean we invoke the query to evaluate the static
+        // with different parameter environments, thus causing the static to be evaluated
+        // multiple times.
+        let param_env = if self.tcx.is_static(gid.instance.def_id()) {
+            ty::ParamEnv::reveal_all()
         } else {
-            self.tcx.const_eval_instance(self.param_env, gid.instance, Some(self.tcx.span))?
+            self.param_env
         };
+        let val = if let Some(promoted) = gid.promoted {
+            self.tcx.const_eval_promoted(param_env, gid.instance, promoted)?
+        } else {
+            self.tcx.const_eval_instance(param_env, gid.instance, Some(self.tcx.span))?
+        };
+
         // Even though `ecx.const_eval` is called from `eval_const_to_op` we can never have a
         // recursion deeper than one level, because the `tcx.const_eval` above is guaranteed to not
         // return `ConstValue::Unevaluated`, which is the only way that `eval_const_to_op` will call

src/librustc_mir/interpret/operand.rs

@@ -537,14 +537,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 // potentially requiring the current static to be evaluated again. This is not a
                 // problem here, because we are building an operand which means an actual read is
                 // happening.
-                // FIXME(oli-obk): eliminate all the `const_eval_raw` usages when we get rid of
-                // `StaticKind` once and for all.
-                // FIXME the following line should have been:
-                // return self.const_eval(GlobalId { instance, promoted });
-                // but since the addition of Promoteds being Constants is causing const validation
-                // cycles. Promoteds being Constants exercise const validation more often and it
-                // may have made show up a pre-existing bug.
-                return Ok(OpTy::from(self.const_eval_raw(GlobalId { instance, promoted })?));
+                return Ok(OpTy::from(self.const_eval(GlobalId { instance, promoted })?));
             }
             ty::ConstKind::Infer(..)
             | ty::ConstKind::Bound(..)

src/test/ui/consts/const-eval/ub-nonnull.stderr

@@ -13,7 +13,7 @@ LL | / const OUT_OF_BOUNDS_PTR: NonNull<u8> = { unsafe {
 LL | |     let ptr: &[u8; 256] = mem::transmute(&0u8); // &0 gets promoted so it does not dangle
 LL | |     // Use address-of-element for pointer arithmetic. This could wrap around to NULL!
 LL | |     let out_of_bounds_ptr = &ptr[255];
-   | |                             ^^^^^^^^^ Memory access failed: pointer must be in-bounds at offset 256, but is outside bounds of allocation 8 which has size 1
+   | |                             ^^^^^^^^^ Memory access failed: pointer must be in-bounds at offset 256, but is outside bounds of allocation 9 which has size 1
 LL | |     mem::transmute(out_of_bounds_ptr)
 LL | | } };
    | |____-