Auto merge of #103917 - oli-obk:layout_math, r=RalfJung,lcnr

Various cleanups around scalar layout restrictions

Pulled out of #103724

Author: bors

compiler/rustc_abi/src/layout.rs

@@ -382,28 +382,26 @@ pub trait LayoutCalculator {
             let (start, end) = scalar_valid_range;
             match st.abi {
                 Abi::Scalar(ref mut scalar) | Abi::ScalarPair(ref mut scalar, _) => {
-                    // the asserts ensure that we are not using the
-                    // `#[rustc_layout_scalar_valid_range(n)]`
-                    // attribute to widen the range of anything as that would probably
-                    // result in UB somewhere
-                    // FIXME(eddyb) the asserts are probably not needed,
-                    // as larger validity ranges would result in missed
+                    // Enlarging validity ranges would result in missed
                     // optimizations, *not* wrongly assuming the inner
-                    // value is valid. e.g. unions enlarge validity ranges,
+                    // value is valid. e.g. unions already enlarge validity ranges,
                     // because the values may be uninitialized.
+                    //
+                    // Because of that we only check that the start and end
+                    // of the range is representable with this scalar type.
+
+                    let max_value = scalar.size(dl).unsigned_int_max();
                     if let Bound::Included(start) = start {
                         // FIXME(eddyb) this might be incorrect - it doesn't
                         // account for wrap-around (end < start) ranges.
-                        let valid_range = scalar.valid_range_mut();
-                        assert!(valid_range.start <= start);
-                        valid_range.start = start;
+                        assert!(start <= max_value, "{start} > {max_value}");
+                        scalar.valid_range_mut().start = start;
                     }
                     if let Bound::Included(end) = end {
                         // FIXME(eddyb) this might be incorrect - it doesn't
                         // account for wrap-around (end < start) ranges.
-                        let valid_range = scalar.valid_range_mut();
-                        assert!(valid_range.end >= end);
-                        valid_range.end = end;
+                        assert!(end <= max_value, "{end} > {max_value}");
+                        scalar.valid_range_mut().end = end;
                     }
 
                     // Update `largest_niche` if we have introduced a larger niche.

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -785,18 +785,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
                 }
             }
             Abi::ScalarPair(a_layout, b_layout) => {
-                // There is no `rustc_layout_scalar_valid_range_start` for pairs, so
-                // we would validate these things as we descend into the fields,
-                // but that can miss bugs in layout computation. Layout computation
-                // is subtle due to enums having ScalarPair layout, where one field
-                // is the discriminant.
-                if cfg!(debug_assertions)
-                    && !a_layout.is_uninit_valid()
-                    && !b_layout.is_uninit_valid()
-                {
-                    // We can only proceed if *both* scalars need to be initialized.
-                    // FIXME: find a way to also check ScalarPair when one side can be uninit but
-                    // the other must be init.
+                // We can only proceed if *both* scalars need to be initialized.
+                // FIXME: find a way to also check ScalarPair when one side can be uninit but
+                // the other must be init.
+                if !a_layout.is_uninit_valid() && !b_layout.is_uninit_valid() {
                     let (a, b) =
                         self.read_immediate(op, "initiailized scalar value")?.to_scalar_pair();
                     self.visit_scalar(a, a_layout)?;

compiler/rustc_const_eval/src/transform/validate.rs

@@ -81,6 +81,7 @@ struct TypeChecker<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
+    #[track_caller]
     fn fail(&self, location: Location, msg: impl AsRef<str>) {
         let span = self.body.source_info(location).span;
         // We use `delay_span_bug` as we might see broken MIR when other errors have already
@@ -226,12 +227,12 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                 let check_equal = |this: &Self, location, f_ty| {
                     if !this.mir_assign_valid_types(ty, f_ty) {
                         this.fail(
-                        location,
-                        format!(
-                            "Field projection `{:?}.{:?}` specified type `{:?}`, but actual type is `{:?}`",
-                            parent, f, ty, f_ty
+                            location,
+                            format!(
+                                "Field projection `{:?}.{:?}` specified type `{:?}`, but actual type is `{:?}`",
+                                parent, f, ty, f_ty
+                            )
                         )
-                    )
                     }
                 };
 

compiler/rustc_lint/src/builtin.rs

@@ -52,7 +52,7 @@ use rustc_span::edition::Edition;
 use rustc_span::source_map::Spanned;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{BytePos, InnerSpan, Span};
-use rustc_target::abi::VariantIdx;
+use rustc_target::abi::{Abi, VariantIdx};
 use rustc_trait_selection::traits::{self, misc::can_type_implement_copy};
 
 use crate::nonstandard_style::{method_context, MethodLateContext};
@@ -2413,8 +2413,34 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
         }
 
         /// Information about why a type cannot be initialized this way.
-        /// Contains an error message and optionally a span to point at.
-        type InitError = (String, Option<Span>);
+        struct InitError {
+            message: String,
+            /// Spans from struct fields and similar that can be obtained from just the type.
+            span: Option<Span>,
+            /// Used to report a trace through adts.
+            nested: Option<Box<InitError>>,
+        }
+        impl InitError {
+            fn spanned(self, span: Span) -> InitError {
+                Self { span: Some(span), ..self }
+            }
+
+            fn nested(self, nested: impl Into<Option<InitError>>) -> InitError {
+                assert!(self.nested.is_none());
+                Self { nested: nested.into().map(Box::new), ..self }
+            }
+        }
+
+        impl<'a> From<&'a str> for InitError {
+            fn from(s: &'a str) -> Self {
+                s.to_owned().into()
+            }
+        }
+        impl From<String> for InitError {
+            fn from(message: String) -> Self {
+                Self { message, span: None, nested: None }
+            }
+        }
 
         /// Test if this constant is all-0.
         fn is_zero(expr: &hir::Expr<'_>) -> bool {
@@ -2470,25 +2496,54 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
 
         fn variant_find_init_error<'tcx>(
             cx: &LateContext<'tcx>,
+            ty: Ty<'tcx>,
             variant: &VariantDef,
             substs: ty::SubstsRef<'tcx>,
             descr: &str,
             init: InitKind,
         ) -> Option<InitError> {
-            variant.fields.iter().find_map(|field| {
-                ty_find_init_error(cx, field.ty(cx.tcx, substs), init).map(|(mut msg, span)| {
-                    if span.is_none() {
-                        // Point to this field, should be helpful for figuring
-                        // out where the source of the error is.
-                        let span = cx.tcx.def_span(field.did);
-                        write!(&mut msg, " (in this {descr})").unwrap();
-                        (msg, Some(span))
+            let mut field_err = variant.fields.iter().find_map(|field| {
+                ty_find_init_error(cx, field.ty(cx.tcx, substs), init).map(|mut err| {
+                    if !field.did.is_local() {
+                        err
+                    } else if err.span.is_none() {
+                        err.span = Some(cx.tcx.def_span(field.did));
+                        write!(&mut err.message, " (in this {descr})").unwrap();
+                        err
                     } else {
-                        // Just forward.
-                        (msg, span)
+                        InitError::from(format!("in this {descr}"))
+                            .spanned(cx.tcx.def_span(field.did))
+                            .nested(err)
                     }
                 })
-            })
+            });
+
+            // Check if this ADT has a constrained layout (like `NonNull` and friends).
+            if let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty)) {
+                if let Abi::Scalar(scalar) | Abi::ScalarPair(scalar, _) = &layout.abi {
+                    let range = scalar.valid_range(cx);
+                    let msg = if !range.contains(0) {
+                        "must be non-null"
+                    } else if init == InitKind::Uninit && !scalar.is_always_valid(cx) {
+                        // Prefer reporting on the fields over the entire struct for uninit,
+                        // as the information bubbles out and it may be unclear why the type can't
+                        // be null from just its outside signature.
+
+                        "must be initialized inside its custom valid range"
+                    } else {
+                        return field_err;
+                    };
+                    if let Some(field_err) = &mut field_err {
+                        // Most of the time, if the field error is the same as the struct error,
+                        // the struct error only happens because of the field error.
+                        if field_err.message.contains(msg) {
+                            field_err.message = format!("because {}", field_err.message);
+                        }
+                    }
+                    return Some(InitError::from(format!("`{ty}` {msg}")).nested(field_err));
+                }
+            }
+            field_err
         }
 
         /// Return `Some` only if we are sure this type does *not*
@@ -2501,63 +2556,36 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
             use rustc_type_ir::sty::TyKind::*;
             match ty.kind() {
                 // Primitive types that don't like 0 as a value.
-                Ref(..) => Some(("references must be non-null".to_string(), None)),
-                Adt(..) if ty.is_box() => Some(("`Box` must be non-null".to_string(), None)),
-                FnPtr(..) => Some(("function pointers must be non-null".to_string(), None)),
-                Never => Some(("the `!` type has no valid value".to_string(), None)),
+                Ref(..) => Some("references must be non-null".into()),
+                Adt(..) if ty.is_box() => Some("`Box` must be non-null".into()),
+                FnPtr(..) => Some("function pointers must be non-null".into()),
+                Never => Some("the `!` type has no valid value".into()),
                 RawPtr(tm) if matches!(tm.ty.kind(), Dynamic(..)) =>
                 // raw ptr to dyn Trait
                 {
-                    Some(("the vtable of a wide raw pointer must be non-null".to_string(), None))
+                    Some("the vtable of a wide raw pointer must be non-null".into())
                 }
                 // Primitive types with other constraints.
                 Bool if init == InitKind::Uninit => {
-                    Some(("booleans must be either `true` or `false`".to_string(), None))
+                    Some("booleans must be either `true` or `false`".into())
                 }
                 Char if init == InitKind::Uninit => {
-                    Some(("characters must be a valid Unicode codepoint".to_string(), None))
+                    Some("characters must be a valid Unicode codepoint".into())
                 }
                 Int(_) | Uint(_) if init == InitKind::Uninit => {
-                    Some(("integers must not be uninitialized".to_string(), None))
-                }
-                Float(_) if init == InitKind::Uninit => {
-                    Some(("floats must not be uninitialized".to_string(), None))
+                    Some("integers must be initialized".into())
                 }
+                Float(_) if init == InitKind::Uninit => Some("floats must be initialized".into()),
                 RawPtr(_) if init == InitKind::Uninit => {
-                    Some(("raw pointers must not be uninitialized".to_string(), None))
+                    Some("raw pointers must be initialized".into())
                 }
                 // Recurse and checks for some compound types. (but not unions)
                 Adt(adt_def, substs) if !adt_def.is_union() => {
-                    // First check if this ADT has a layout attribute (like `NonNull` and friends).
-                    use std::ops::Bound;
-                    match cx.tcx.layout_scalar_valid_range(adt_def.did()) {
-                        // We exploit here that `layout_scalar_valid_range` will never
-                        // return `Bound::Excluded`.  (And we have tests checking that we
-                        // handle the attribute correctly.)
-                        // We don't add a span since users cannot declare such types anyway.
-                        (Bound::Included(lo), Bound::Included(hi)) if 0 < lo && lo < hi => {
-                            return Some((format!("`{}` must be non-null", ty), None));
-                        }
-                        (Bound::Included(lo), Bound::Unbounded) if 0 < lo => {
-                            return Some((format!("`{}` must be non-null", ty), None));
-                        }
-                        (Bound::Included(_), _) | (_, Bound::Included(_))
-                            if init == InitKind::Uninit =>
-                        {
-                            return Some((
-                                format!(
-                                    "`{}` must be initialized inside its custom valid range",
-                                    ty,
-                                ),
-                                None,
-                            ));
-                        }
-                        _ => {}
-                    }
                     // Handle structs.
                     if adt_def.is_struct() {
                         return variant_find_init_error(
                             cx,
+                            ty,
                             adt_def.non_enum_variant(),
                             substs,
                             "struct field",
@@ -2581,13 +2609,14 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
                         Some((variant, definitely_inhabited))
                     });
                     let Some(first_variant) = potential_variants.next() else {
-                        return Some(("enums with no inhabited variants have no valid value".to_string(), Some(span)));
+                        return Some(InitError::from("enums with no inhabited variants have no valid value").spanned(span));
                     };
                     // So we have at least one potentially inhabited variant. Might we have two?
                     let Some(second_variant) = potential_variants.next() else {
                         // There is only one potentially inhabited variant. So we can recursively check that variant!
                         return variant_find_init_error(
                             cx,
+                            ty,
                             &first_variant.0,
                             substs,
                             "field of the only potentially inhabited enum variant",
@@ -2605,10 +2634,9 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
                                 .filter(|(_variant, definitely_inhabited)| *definitely_inhabited)
                                 .count();
                         if definitely_inhabited > 1 {
-                            return Some((
-                                "enums with multiple inhabited variants have to be initialized to a variant".to_string(),
-                                Some(span),
-                            ));
+                            return Some(InitError::from(
+                                "enums with multiple inhabited variants have to be initialized to a variant",
+                            ).spanned(span));
                         }
                     }
                     // We couldn't find anything wrong here.
@@ -2637,8 +2665,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
             // using zeroed or uninitialized memory.
             // We are extremely conservative with what we warn about.
             let conjured_ty = cx.typeck_results().expr_ty(expr);
-            if let Some((msg, span)) =
-                with_no_trimmed_paths!(ty_find_init_error(cx, conjured_ty, init))
+            if let Some(mut err) = with_no_trimmed_paths!(ty_find_init_error(cx, conjured_ty, init))
             {
                 // FIXME(davidtwco): make translatable
                 cx.struct_span_lint(
@@ -2664,10 +2691,17 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
                             "help: use `MaybeUninit<T>` instead, \
                             and only call `assume_init` after initialization is done",
                         );
-                        if let Some(span) = span {
-                            lint.span_note(span, &msg);
-                        } else {
-                            lint.note(&msg);
+                        loop {
+                            if let Some(span) = err.span {
+                                lint.span_note(span, &err.message);
+                            } else {
+                                lint.note(&err.message);
+                            }
+                            if let Some(e) = err.nested {
+                                err = *e;
+                            } else {
+                                break;
+                            }
                         }
                         lint
                     },

compiler/rustc_middle/src/ty/consts/int.rs

@@ -245,6 +245,18 @@ impl ScalarInt {
         self.to_bits(size)
     }
 
+    // Tries to convert the `ScalarInt` to `bool`. Fails if the `size` of the `ScalarInt`
+    // in not equal to `Size { raw: 1 }` or if the value is not 0 or 1 and returns the `size`
+    // value of the `ScalarInt` in that case.
+    #[inline]
+    pub fn try_to_bool(self) -> Result<bool, Size> {
+        match self.try_to_u8()? {
+            0 => Ok(false),
+            1 => Ok(true),
+            _ => Err(self.size()),
+        }
+    }
+
     // Tries to convert the `ScalarInt` to `u8`. Fails if the `size` of the `ScalarInt`
     // in not equal to `Size { raw: 1 }` and returns the `size` value of the `ScalarInt` in
     // that case.

compiler/rustc_span/src/def_id.rs

@@ -34,7 +34,7 @@ impl CrateNum {
 
 impl fmt::Display for CrateNum {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        fmt::Display::fmt(&self.private, f)
+        fmt::Display::fmt(&self.as_u32(), f)
     }
 }
 

compiler/rustc_span/src/hygiene.rs

@@ -76,7 +76,7 @@ pub struct ExpnId {
 impl fmt::Debug for ExpnId {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         // Generate crate_::{{expn_}}.
-        write!(f, "{:?}::{{{{expn{}}}}}", self.krate, self.local_id.private)
+        write!(f, "{:?}::{{{{expn{}}}}}", self.krate, self.local_id.as_u32())
     }
 }