Auto merge of rust-lang#95291 - Dylan-DPC:rollup-vrb4wlw, r=Dylan-DPC

Rollup of 5 pull requests

Successful merges:

 - rust-lang#94391 (Fix ice when error reporting recursion errors)
 - rust-lang#94655 (Clarify which kinds of MIR are allowed during which phases.)
 - rust-lang#95179 (Try to evaluate in try unify and postpone resolution of constants that contain inference variables)
 - rust-lang#95270 (debuginfo: Fix debuginfo for Box<T> where T is unsized.)
 - rust-lang#95276 (add diagnostic items for clippy's `trim_split_whitespace`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

@@ -166,6 +166,13 @@ fn build_pointer_or_reference_di_node<'ll, 'tcx>(
     pointee_type: Ty<'tcx>,
     unique_type_id: UniqueTypeId<'tcx>,
 ) -> DINodeCreationResult<'ll> {
+    // The debuginfo generated by this function is only valid if `ptr_type` is really just
+    // a (fat) pointer. Make sure it is not called for e.g. `Box<T, NonZSTAllocator>`.
+    debug_assert_eq!(
+        cx.size_and_align_of(ptr_type),
+        cx.size_and_align_of(cx.tcx.mk_mut_ptr(pointee_type))
+    );
+
     let pointee_type_di_node = type_di_node(cx, pointee_type);
 
     return_if_di_node_created_in_meantime!(cx, unique_type_id);
@@ -212,7 +219,17 @@ fn build_pointer_or_reference_di_node<'ll, 'tcx>(
                     DIFlags::FlagZero,
                 ),
                 |cx, owner| {
-                    let layout = cx.layout_of(ptr_type);
+                    // FIXME: If this fat pointer is a `Box` then we don't want to use its
+                    //        type layout and instead use the layout of the raw pointer inside
+                    //        of it.
+                    //        The proper way to handle this is to not treat Box as a pointer
+                    //        at all and instead emit regular struct debuginfo for it. We just
+                    //        need to make sure that we don't break existing debuginfo consumers
+                    //        by doing that (at least not without a warning period).
+                    let layout_type =
+                        if ptr_type.is_box() { cx.tcx.mk_mut_ptr(pointee_type) } else { ptr_type };
+
+                    let layout = cx.layout_of(layout_type);
                     let addr_field = layout.field(cx, abi::FAT_PTR_ADDR);
                     let extra_field = layout.field(cx, abi::FAT_PTR_EXTRA);
 

compiler/rustc_const_eval/src/transform/promote_consts.rs

@@ -42,7 +42,7 @@ pub struct PromoteTemps<'tcx> {
 
 impl<'tcx> MirPass<'tcx> for PromoteTemps<'tcx> {
     fn phase_change(&self) -> Option<MirPhase> {
-        Some(MirPhase::ConstPromotion)
+        Some(MirPhase::ConstsPromoted)
     }
 
     fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {

compiler/rustc_const_eval/src/transform/validate.rs

@@ -266,30 +266,23 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                             );
                         }
                     }
-                    // The deaggregator currently does not deaggreagate arrays.
-                    // So for now, we ignore them here.
-                    Rvalue::Aggregate(box AggregateKind::Array { .. }, _) => {}
-                    // All other aggregates must be gone after some phases.
-                    Rvalue::Aggregate(box kind, _) => {
-                        if self.mir_phase > MirPhase::DropLowering
-                            && !matches!(kind, AggregateKind::Generator(..))
-                        {
-                            // Generators persist until the state machine transformation, but all
-                            // other aggregates must have been lowered.
-                            self.fail(
-                                location,
-                                format!("{:?} have been lowered to field assignments", rvalue),
-                            )
-                        } else if self.mir_phase > MirPhase::GeneratorLowering {
-                            // No more aggregates after drop and generator lowering.
+                    Rvalue::Aggregate(agg_kind, _) => {
+                        let disallowed = match **agg_kind {
+                            AggregateKind::Array(..) => false,
+                            AggregateKind::Generator(..) => {
+                                self.mir_phase >= MirPhase::GeneratorsLowered
+                            }
+                            _ => self.mir_phase >= MirPhase::Deaggregated,
+                        };
+                        if disallowed {
                             self.fail(
                                 location,
                                 format!("{:?} have been lowered to field assignments", rvalue),
                             )
                         }
                     }
                     Rvalue::Ref(_, BorrowKind::Shallow, _) => {
-                        if self.mir_phase > MirPhase::DropLowering {
+                        if self.mir_phase >= MirPhase::DropsLowered {
                             self.fail(
                                 location,
                                 "`Assign` statement with a `Shallow` borrow should have been removed after drop lowering phase",
@@ -300,15 +293,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                 }
             }
             StatementKind::AscribeUserType(..) => {
-                if self.mir_phase > MirPhase::DropLowering {
+                if self.mir_phase >= MirPhase::DropsLowered {
                     self.fail(
                         location,
                         "`AscribeUserType` should have been removed after drop lowering phase",
                     );
                 }
             }
             StatementKind::FakeRead(..) => {
-                if self.mir_phase > MirPhase::DropLowering {
+                if self.mir_phase >= MirPhase::DropsLowered {
                     self.fail(
                         location,
                         "`FakeRead` should have been removed after drop lowering phase",
@@ -351,10 +344,18 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                     self.fail(location, format!("bad arg ({:?} != usize)", op_cnt_ty))
                 }
             }
-            StatementKind::SetDiscriminant { .. }
-            | StatementKind::StorageLive(..)
+            StatementKind::SetDiscriminant { .. } => {
+                if self.mir_phase < MirPhase::DropsLowered {
+                    self.fail(location, "`SetDiscriminant` is not allowed until drop elaboration");
+                }
+            }
+            StatementKind::Retag(_, _) => {
+                // FIXME(JakobDegen) The validator should check that `self.mir_phase <
+                // DropsLowered`. However, this causes ICEs with generation of drop shims, which
+                // seem to fail to set their `MirPhase` correctly.
+            }
+            StatementKind::StorageLive(..)
             | StatementKind::StorageDead(..)
-            | StatementKind::Retag(_, _)
             | StatementKind::Coverage(_)
             | StatementKind::Nop => {}
         }
@@ -424,10 +425,10 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                 }
             }
             TerminatorKind::DropAndReplace { target, unwind, .. } => {
-                if self.mir_phase > MirPhase::DropLowering {
+                if self.mir_phase >= MirPhase::DropsLowered {
                     self.fail(
                         location,
-                        "`DropAndReplace` is not permitted to exist after drop elaboration",
+                        "`DropAndReplace` should have been removed during drop elaboration",
                     );
                 }
                 self.check_edge(location, *target, EdgeKind::Normal);
@@ -494,7 +495,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                 }
             }
             TerminatorKind::Yield { resume, drop, .. } => {
-                if self.mir_phase > MirPhase::GeneratorLowering {
+                if self.mir_phase >= MirPhase::GeneratorsLowered {
                     self.fail(location, "`Yield` should have been replaced by generator lowering");
                 }
                 self.check_edge(location, *resume, EdgeKind::Normal);
@@ -503,10 +504,22 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                 }
             }
             TerminatorKind::FalseEdge { real_target, imaginary_target } => {
+                if self.mir_phase >= MirPhase::DropsLowered {
+                    self.fail(
+                        location,
+                        "`FalseEdge` should have been removed after drop elaboration",
+                    );
+                }
                 self.check_edge(location, *real_target, EdgeKind::Normal);
                 self.check_edge(location, *imaginary_target, EdgeKind::Normal);
             }
             TerminatorKind::FalseUnwind { real_target, unwind } => {
+                if self.mir_phase >= MirPhase::DropsLowered {
+                    self.fail(
+                        location,
+                        "`FalseUnwind` should have been removed after drop elaboration",
+                    );
+                }
                 self.check_edge(location, *real_target, EdgeKind::Normal);
                 if let Some(unwind) = unwind {
                     self.check_edge(location, *unwind, EdgeKind::Unwind);
@@ -520,12 +533,19 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                     self.check_edge(location, *cleanup, EdgeKind::Unwind);
                 }
             }
+            TerminatorKind::GeneratorDrop => {
+                if self.mir_phase >= MirPhase::GeneratorsLowered {
+                    self.fail(
+                        location,
+                        "`GeneratorDrop` should have been replaced by generator lowering",
+                    );
+                }
+            }
             // Nothing to validate for these.
             TerminatorKind::Resume
             | TerminatorKind::Abort
             | TerminatorKind::Return
-            | TerminatorKind::Unreachable
-            | TerminatorKind::GeneratorDrop => {}
+            | TerminatorKind::Unreachable => {}
         }
 
         self.super_terminator(terminator, location);

compiler/rustc_infer/src/infer/mod.rs

@@ -20,8 +20,7 @@ use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_middle::infer::canonical::{Canonical, CanonicalVarValues};
 use rustc_middle::infer::unify_key::{ConstVarValue, ConstVariableValue};
 use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind, ToType};
-use rustc_middle::mir::interpret::ErrorHandled;
-use rustc_middle::mir::interpret::EvalToConstValueResult;
+use rustc_middle::mir::interpret::{ErrorHandled, EvalToConstValueResult};
 use rustc_middle::traits::select;
 use rustc_middle::ty::error::{ExpectedFound, TypeError};
 use rustc_middle::ty::fold::{TypeFoldable, TypeFolder};
@@ -71,7 +70,6 @@ mod sub;
 pub mod type_variable;
 mod undo_log;
 
-use crate::infer::canonical::OriginalQueryValues;
 pub use rustc_middle::infer::unify_key;
 
 #[must_use]
@@ -687,15 +685,28 @@ pub struct CombinedSnapshot<'a, 'tcx> {
 impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     /// calls `tcx.try_unify_abstract_consts` after
     /// canonicalizing the consts.
+    #[instrument(skip(self), level = "debug")]
     pub fn try_unify_abstract_consts(
         &self,
         a: ty::Unevaluated<'tcx, ()>,
         b: ty::Unevaluated<'tcx, ()>,
+        param_env: ty::ParamEnv<'tcx>,
     ) -> bool {
-        let canonical = self.canonicalize_query((a, b), &mut OriginalQueryValues::default());
-        debug!("canonical consts: {:?}", &canonical.value);
+        // Reject any attempt to unify two unevaluated constants that contain inference
+        // variables, since inference variables in queries lead to ICEs.
+        if a.substs.has_infer_types_or_consts()
+            || b.substs.has_infer_types_or_consts()
+            || param_env.has_infer_types_or_consts()
+        {
+            debug!("a or b or param_env contain infer vars in its substs -> cannot unify");
+            return false;
+        }
+
+        let param_env_and = param_env.and((a, b));
+        let erased = self.tcx.erase_regions(param_env_and);
+        debug!("after erase_regions: {:?}", erased);
 
-        self.tcx.try_unify_abstract_consts(canonical.value)
+        self.tcx.try_unify_abstract_consts(erased)
     }
 
     pub fn is_in_snapshot(&self) -> bool {
@@ -1598,22 +1609,27 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     ///
     /// This handles inferences variables within both `param_env` and `substs` by
     /// performing the operation on their respective canonical forms.
+    #[instrument(skip(self), level = "debug")]
     pub fn const_eval_resolve(
         &self,
         param_env: ty::ParamEnv<'tcx>,
         unevaluated: ty::Unevaluated<'tcx>,
         span: Option<Span>,
     ) -> EvalToConstValueResult<'tcx> {
         let substs = self.resolve_vars_if_possible(unevaluated.substs);
+        debug!(?substs);
 
         // Postpone the evaluation of constants whose substs depend on inference
         // variables
         if substs.has_infer_types_or_consts() {
+            debug!("substs have infer types or consts: {:?}", substs);
             return Err(ErrorHandled::TooGeneric);
         }
 
         let param_env_erased = self.tcx.erase_regions(param_env);
         let substs_erased = self.tcx.erase_regions(substs);
+        debug!(?param_env_erased);
+        debug!(?substs_erased);
 
         let unevaluated = ty::Unevaluated {
             def: unevaluated.def,

compiler/rustc_middle/src/mir/interpret/queries.rs

@@ -1,6 +1,7 @@
 use super::{ErrorHandled, EvalToConstValueResult, GlobalId};
 
 use crate::mir;
+use crate::ty::fold::TypeFoldable;
 use crate::ty::subst::InternalSubsts;
 use crate::ty::{self, TyCtxt};
 use rustc_hir::def_id::DefId;
@@ -38,6 +39,16 @@ impl<'tcx> TyCtxt<'tcx> {
         ct: ty::Unevaluated<'tcx>,
         span: Option<Span>,
     ) -> EvalToConstValueResult<'tcx> {
+        // Cannot resolve `Unevaluated` constants that contain inference
+        // variables. We reject those here since `resolve_opt_const_arg`
+        // would fail otherwise.
+        //
+        // When trying to evaluate constants containing inference variables,
+        // use `Infcx::const_eval_resolve` instead.
+        if ct.substs.has_infer_types_or_consts() {
+            bug!("did not expect inference variables here");
+        }
+
         match ty::Instance::resolve_opt_const_arg(self, param_env, ct.def, ct.substs) {
             Ok(Some(instance)) => {
                 let cid = GlobalId { instance, promoted: ct.promoted };