Auto merge of rust-lang#154326 - JonathanBrouwer:rollup-MflIdQW, r=JonathanBrouwer

Rollup of 5 pull requests

Successful merges:

 - rust-lang#152710 (Unalign `PackedFingerprint` on all hosts, not just x86 and x86-64)
 - rust-lang#153874 (constify const Fn*: Destruct)
 - rust-lang#154097 (improve validation error messages: show surrounding type)
 - rust-lang#154277 (use `minicore` more in testing inline assembly)
 - rust-lang#154293 (Use verbose span suggestion for type const)

Author: bors

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -425,7 +425,7 @@ fn const_validate_mplace<'tcx>(
     cid: GlobalId<'tcx>,
 ) -> Result<(), ErrorHandled> {
     let alloc_id = mplace.ptr().provenance.unwrap().alloc_id();
-    let mut ref_tracking = RefTracking::new(mplace.clone());
+    let mut ref_tracking = RefTracking::new(mplace.clone(), mplace.layout.ty);
     let mut inner = false;
     while let Some((mplace, path)) = ref_tracking.next() {
         let mode = match ecx.tcx.static_mutability(cid.instance.def_id()) {

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -47,9 +47,9 @@ use super::UnsupportedOpInfo::*;
 macro_rules! err_validation_failure {
     ($where:expr,  $msg:expr ) => {{
         let where_ = &$where;
-        let path = if !where_.is_empty() {
+        let path = if !where_.projs.is_empty() {
             let mut path = String::new();
-            write_path(&mut path, where_);
+            write_path(&mut path, &where_.projs);
             Some(path)
         } else {
             None
@@ -59,6 +59,7 @@ macro_rules! err_validation_failure {
         use ValidationErrorKind::*;
         let msg = ValidationErrorKind::from($msg);
         err_ub!(ValidationError {
+            orig_ty: where_.orig_ty,
             path,
             ptr_bytes_warning: msg.ptr_bytes_warning(),
             msg: msg.to_string(),
@@ -234,7 +235,7 @@ fn fmt_range(r: WrappingRange, max_hi: u128) -> String {
 /// So we track a `Vec<PathElem>` where `PathElem` contains all the data we
 /// need to later print something for the user.
 #[derive(Copy, Clone, Debug)]
-pub enum PathElem {
+pub enum PathElem<'tcx> {
     Field(Symbol),
     Variant(Symbol),
     CoroutineState(VariantIdx),
@@ -244,10 +245,22 @@ pub enum PathElem {
     Deref,
     EnumTag,
     CoroutineTag,
-    DynDowncast,
+    DynDowncast(Ty<'tcx>),
     Vtable,
 }
 
+#[derive(Clone, Debug)]
+pub struct Path<'tcx> {
+    orig_ty: Ty<'tcx>,
+    projs: Vec<PathElem<'tcx>>,
+}
+
+impl<'tcx> Path<'tcx> {
+    fn new(ty: Ty<'tcx>) -> Self {
+        Self { orig_ty: ty, projs: vec![] }
+    }
+}
+
 /// Extra things to check for during validation of CTFE results.
 #[derive(Copy, Clone)]
 pub enum CtfeValidationMode {
@@ -280,16 +293,10 @@ pub struct RefTracking<T, PATH = ()> {
     todo: Vec<(T, PATH)>,
 }
 
-impl<T: Clone + Eq + Hash + std::fmt::Debug, PATH: Default> RefTracking<T, PATH> {
+impl<T: Clone + Eq + Hash + std::fmt::Debug, PATH> RefTracking<T, PATH> {
     pub fn empty() -> Self {
         RefTracking { seen: FxHashSet::default(), todo: vec![] }
     }
-    pub fn new(val: T) -> Self {
-        let mut ref_tracking_for_consts =
-            RefTracking { seen: FxHashSet::default(), todo: vec![(val.clone(), PATH::default())] };
-        ref_tracking_for_consts.seen.insert(val);
-        ref_tracking_for_consts
-    }
     pub fn next(&mut self) -> Option<(T, PATH)> {
         self.todo.pop()
     }
@@ -304,8 +311,17 @@ impl<T: Clone + Eq + Hash + std::fmt::Debug, PATH: Default> RefTracking<T, PATH>
     }
 }
 
+impl<'tcx, T: Clone + Eq + Hash + std::fmt::Debug> RefTracking<T, Path<'tcx>> {
+    pub fn new(val: T, ty: Ty<'tcx>) -> Self {
+        let mut ref_tracking_for_consts =
+            RefTracking { seen: FxHashSet::default(), todo: vec![(val.clone(), Path::new(ty))] };
+        ref_tracking_for_consts.seen.insert(val);
+        ref_tracking_for_consts
+    }
+}
+
 /// Format a path
-fn write_path(out: &mut String, path: &[PathElem]) {
+fn write_path(out: &mut String, path: &[PathElem<'_>]) {
     use self::PathElem::*;
 
     for elem in path.iter() {
@@ -323,7 +339,7 @@ fn write_path(out: &mut String, path: &[PathElem]) {
             // even use the usual syntax because we are just showing the projections,
             // not the root.
             Deref => write!(out, ".<deref>"),
-            DynDowncast => write!(out, ".<dyn-downcast>"),
+            DynDowncast(ty) => write!(out, ".<dyn-downcast({ty})>"),
             Vtable => write!(out, ".<vtable>"),
         }
         .unwrap()
@@ -382,10 +398,9 @@ impl RangeSet {
 
 struct ValidityVisitor<'rt, 'tcx, M: Machine<'tcx>> {
     /// The `path` may be pushed to, but the part that is present when a function
-    /// starts must not be changed!  `visit_fields` and `visit_array` rely on
-    /// this stack discipline.
-    path: Vec<PathElem>,
-    ref_tracking: Option<&'rt mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Vec<PathElem>>>,
+    /// starts must not be changed!  `with_elem` relies on this stack discipline.
+    path: Path<'tcx>,
+    ref_tracking: Option<&'rt mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Path<'tcx>>>,
     /// `None` indicates this is not validating for CTFE (but for runtime).
     ctfe_mode: Option<CtfeValidationMode>,
     ecx: &'rt mut InterpCx<'tcx, M>,
@@ -404,7 +419,12 @@ struct ValidityVisitor<'rt, 'tcx, M: Machine<'tcx>> {
 }
 
 impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
-    fn aggregate_field_path_elem(&mut self, layout: TyAndLayout<'tcx>, field: usize) -> PathElem {
+    fn aggregate_field_path_elem(
+        &mut self,
+        layout: TyAndLayout<'tcx>,
+        field: usize,
+        field_ty: Ty<'tcx>,
+    ) -> PathElem<'tcx> {
         // First, check if we are projecting to a variant.
         match layout.variants {
             Variants::Multiple { tag_field, .. } => {
@@ -474,7 +494,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
             // dyn traits
             ty::Dynamic(..) => {
                 assert_eq!(field, 0);
-                PathElem::DynDowncast
+                PathElem::DynDowncast(field_ty)
             }
 
             // nothing else has an aggregate layout
@@ -484,17 +504,17 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
 
     fn with_elem<R>(
         &mut self,
-        elem: PathElem,
+        elem: PathElem<'tcx>,
         f: impl FnOnce(&mut Self) -> InterpResult<'tcx, R>,
     ) -> InterpResult<'tcx, R> {
         // Remember the old state
-        let path_len = self.path.len();
+        let path_len = self.path.projs.len();
         // Record new element
-        self.path.push(elem);
+        self.path.projs.push(elem);
         // Perform operation
         let r = f(self)?;
         // Undo changes
-        self.path.truncate(path_len);
+        self.path.projs.truncate(path_len);
         // Done
         interp_ok(r)
     }
@@ -796,10 +816,10 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
             ref_tracking.track(place, || {
                 // We need to clone the path anyway, make sure it gets created
                 // with enough space for the additional `Deref`.
-                let mut new_path = Vec::with_capacity(path.len() + 1);
-                new_path.extend(path);
-                new_path.push(PathElem::Deref);
-                new_path
+                let mut new_projs = Vec::with_capacity(path.projs.len() + 1);
+                new_projs.extend(&path.projs);
+                new_projs.push(PathElem::Deref);
+                Path { projs: new_projs, orig_ty: path.orig_ty }
             });
         }
         interp_ok(())
@@ -1220,7 +1240,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
         field: usize,
         new_val: &PlaceTy<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx> {
-        let elem = self.aggregate_field_path_elem(old_val.layout, field);
+        let elem = self.aggregate_field_path_elem(old_val.layout, field, new_val.layout.ty);
         self.with_elem(elem, move |this| this.visit_value(new_val))
     }
 
@@ -1385,7 +1405,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
                                 access.bad.start.bytes() / layout.size.bytes(),
                             )
                             .unwrap();
-                            self.path.push(PathElem::ArrayElem(i));
+                            self.path.projs.push(PathElem::ArrayElem(i));
 
                             if matches!(kind, Ub(InvalidUninitBytes(_))) {
                                 err_validation_failure!(self.path, Uninit { expected })
@@ -1515,8 +1535,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
     fn validate_operand_internal(
         &mut self,
         val: &PlaceTy<'tcx, M::Provenance>,
-        path: Vec<PathElem>,
-        ref_tracking: Option<&mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Vec<PathElem>>>,
+        path: Path<'tcx>,
+        ref_tracking: Option<&mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Path<'tcx>>>,
         ctfe_mode: Option<CtfeValidationMode>,
         reset_provenance_and_padding: bool,
     ) -> InterpResult<'tcx> {
@@ -1569,8 +1589,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
     pub(crate) fn const_validate_operand(
         &mut self,
         val: &PlaceTy<'tcx, M::Provenance>,
-        path: Vec<PathElem>,
-        ref_tracking: &mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Vec<PathElem>>,
+        path: Path<'tcx>,
+        ref_tracking: &mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Path<'tcx>>,
         ctfe_mode: CtfeValidationMode,
     ) -> InterpResult<'tcx> {
         self.validate_operand_internal(
@@ -1599,14 +1619,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             reset_provenance_and_padding,
             ?val,
         );
-
         // Note that we *could* actually be in CTFE here with `-Zextra-const-ub-checks`, but it's
         // still correct to not use `ctfe_mode`: that mode is for validation of the final constant
         // value, it rules out things like `UnsafeCell` in awkward places.
         if !recursive {
             return self.validate_operand_internal(
                 val,
-                vec![],
+                Path::new(val.layout.ty),
                 None,
                 None,
                 reset_provenance_and_padding,
@@ -1616,7 +1635,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         let mut ref_tracking = RefTracking::empty();
         self.validate_operand_internal(
             val,
-            vec![],
+            Path::new(val.layout.ty),
             Some(&mut ref_tracking),
             None,
             reset_provenance_and_padding,

compiler/rustc_data_structures/src/fingerprint.rs

@@ -181,22 +181,24 @@ impl<D: Decoder> Decodable<D> for Fingerprint {
     }
 }
 
-// `PackedFingerprint` wraps a `Fingerprint`. Its purpose is to, on certain
-// architectures, behave like a `Fingerprint` without alignment requirements.
-// This behavior is only enabled on x86 and x86_64, where the impact of
-// unaligned accesses is tolerable in small doses.
-//
-// This may be preferable to use in large collections of structs containing
-// fingerprints, as it can reduce memory consumption by preventing the padding
-// that the more strictly-aligned `Fingerprint` can introduce. An application of
-// this is in the query dependency graph, which contains a large collection of
-// `DepNode`s. As of this writing, the size of a `DepNode` decreases by ~30%
-// (from 24 bytes to 17) by using the packed representation here, which
-// noticeably decreases total memory usage when compiling large crates.
-//
-// The wrapped `Fingerprint` is private to reduce the chance of a client
-// invoking undefined behavior by taking a reference to the packed field.
-#[cfg_attr(any(target_arch = "x86", target_arch = "x86_64"), repr(packed))]
+/// `PackedFingerprint` wraps a `Fingerprint`.
+/// Its purpose is to behave like a `Fingerprint` without alignment requirements.
+///
+/// This may be preferable to use in large collections of structs containing
+/// fingerprints, as it can reduce memory consumption by preventing the padding
+/// that the more strictly-aligned `Fingerprint` can introduce. An application of
+/// this is in the query dependency graph, which contains a large collection of
+/// `DepNode`s. As of this writing, the size of a `DepNode` decreases by 25%
+/// (from 24 bytes to 18) by using the packed representation here, which
+/// noticeably decreases total memory usage when compiling large crates.
+///
+/// (Unalignment was previously restricted to `x86` and `x86_64` hosts, but is
+/// now enabled by default for all host architectures, in the hope that the
+/// memory and cache savings should outweigh any unaligned access penalty.)
+///
+/// The wrapped `Fingerprint` is private to reduce the chance of a client
+/// invoking undefined behavior by taking a reference to the packed field.
+#[repr(packed)]
 #[derive(Eq, PartialEq, Ord, PartialOrd, Debug, Clone, Copy, Hash)]
 pub struct PackedFingerprint(Fingerprint);
 

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

@@ -2947,7 +2947,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             );
             if def_id.is_local() {
                 let name = tcx.def_path_str(def_id);
-                err.span_suggestion(
+                err.span_suggestion_verbose(
                     tcx.def_span(def_id).shrink_to_lo(),
                     format!("add `type` before `const` for `{name}`"),
                     format!("type "),

compiler/rustc_middle/src/dep_graph/dep_node.rs

@@ -412,9 +412,6 @@ mod size_asserts {
     use super::*;
     // tidy-alphabetical-start
     static_assert_size!(DepKind, 2);
-    #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
     static_assert_size!(DepNode, 18);
-    #[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))]
-    static_assert_size!(DepNode, 24);
     // tidy-alphabetical-end
 }

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

@@ -318,7 +318,12 @@ pub enum UndefinedBehaviorInfo<'tcx> {
     /// Free-form case. Only for errors that are never caught! Used by miri
     Ub(String),
     /// Validation error.
-    ValidationError { path: Option<String>, msg: String, ptr_bytes_warning: bool },
+    ValidationError {
+        orig_ty: Ty<'tcx>,
+        path: Option<String>,
+        msg: String,
+        ptr_bytes_warning: bool,
+    },
 
     /// Unreachable code was executed.
     Unreachable,
@@ -457,11 +462,11 @@ impl<'tcx> fmt::Display for UndefinedBehaviorInfo<'tcx> {
         match self {
             Ub(msg) => write!(f, "{msg}"),
 
-            ValidationError { path: None, msg, .. } => {
-                write!(f, "constructing invalid value: {msg}")
+            ValidationError { orig_ty, path: None, msg, .. } => {
+                write!(f, "constructing invalid value of type {orig_ty}: {msg}")
             }
-            ValidationError { path: Some(path), msg, .. } => {
-                write!(f, "constructing invalid value at {path}: {msg}")
+            ValidationError { orig_ty, path: Some(path), msg, .. } => {
+                write!(f, "constructing invalid value of type {orig_ty}: at {path}, {msg}")
             }
 
             Unreachable => write!(f, "entering unreachable code"),

compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs

@@ -797,12 +797,16 @@ pub(in crate::solve) fn const_conditions_for_destruct<I: Interner>(
         | ty::Infer(ty::InferTy::FloatVar(_) | ty::InferTy::IntVar(_))
         | ty::Error(_) => Ok(vec![]),
 
-        // Coroutines and closures could implement `[const] Drop`,
+        // Closures are [const] Destruct when all of their upvars (captures) are [const] Destruct.
+        ty::Closure(_, args) => {
+            let closure_args = args.as_closure();
+            Ok(vec![ty::TraitRef::new(cx, destruct_def_id, [closure_args.tupled_upvars_ty()])])
+        }
+        // Coroutines could implement `[const] Drop`,
         // but they don't really need to right now.
-        ty::Closure(_, _)
-        | ty::CoroutineClosure(_, _)
-        | ty::Coroutine(_, _)
-        | ty::CoroutineWitness(_, _) => Err(NoSolution),
+        ty::CoroutineClosure(_, _) | ty::Coroutine(_, _) | ty::CoroutineWitness(_, _) => {
+            Err(NoSolution)
+        }
 
         // FIXME(unsafe_binders): Unsafe binders could implement `[const] Drop`
         // if their inner type implements it.

compiler/rustc_trait_selection/src/traits/effects.rs

@@ -472,12 +472,17 @@ fn evaluate_host_effect_for_destruct_goal<'tcx>(
         | ty::Infer(ty::InferTy::FloatVar(_) | ty::InferTy::IntVar(_))
         | ty::Error(_) => thin_vec![],
 
-        // Coroutines and closures could implement `[const] Drop`,
+        // Closures are [const] Destruct when all of their upvars (captures) are [const] Destruct.
+        ty::Closure(_, args) => {
+            let closure_args = args.as_closure();
+            thin_vec![ty::TraitRef::new(tcx, destruct_def_id, [closure_args.tupled_upvars_ty()])]
+        }
+
+        // Coroutines could implement `[const] Drop`,
         // but they don't really need to right now.
-        ty::Closure(_, _)
-        | ty::CoroutineClosure(_, _)
-        | ty::Coroutine(_, _)
-        | ty::CoroutineWitness(_, _) => return Err(EvaluationFailure::NoSolution),
+        ty::CoroutineClosure(_, _) | ty::Coroutine(_, _) | ty::CoroutineWitness(_, _) => {
+            return Err(EvaluationFailure::NoSolution);
+        }
 
         // FIXME(unsafe_binders): Unsafe binders could implement `[const] Drop`
         // if their inner type implements it.