Auto merge of #144440 - matthiaskrgr:rollup-peb88gb, r=matthiaskrgr

Rollup of 12 pull requests

Successful merges:

 - #142569 (Suggest clone in user-write-code instead of inside macro)
 - #143401 (tests: Don't check for self-printed output in std-backtrace.rs test)
 - #143424 (clippy fix: rely on autoderef)
 - #143970 (Update core::mem::copy documentation)
 - #143979 (Test fixes for Arm64EC Windows)
 - #144200 (Tweak output for non-`Clone` values moved into closures)
 - #144209 (Don't emit two `assume`s in transmutes when one is a subset of the other)
 - #144314 (Hint that choose_pivot returns index in bounds)
 - #144340 (UI test suite clarity changes: Rename `tests/ui/SUMMARY.md` and update rustc dev guide on `error-pattern`)
 - #144368 (resolve: Remove `Scope::CrateRoot`)
 - #144390 (Remove dead code and extend test coverage and diagnostics around it)
 - #144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`)

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

Author: bors

compiler/rustc_abi/src/layout.rs

@@ -313,7 +313,6 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
         scalar_valid_range: (Bound<u128>, Bound<u128>),
         discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool),
         discriminants: impl Iterator<Item = (VariantIdx, i128)>,
-        dont_niche_optimize_enum: bool,
         always_sized: bool,
     ) -> LayoutCalculatorResult<FieldIdx, VariantIdx, F> {
         let (present_first, present_second) = {
@@ -352,13 +351,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
             // structs. (We have also handled univariant enums
             // that allow representation optimization.)
             assert!(is_enum);
-            self.layout_of_enum(
-                repr,
-                variants,
-                discr_range_of_repr,
-                discriminants,
-                dont_niche_optimize_enum,
-            )
+            self.layout_of_enum(repr, variants, discr_range_of_repr, discriminants)
         }
     }
 
@@ -599,7 +592,6 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
         variants: &IndexSlice<VariantIdx, IndexVec<FieldIdx, F>>,
         discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool),
         discriminants: impl Iterator<Item = (VariantIdx, i128)>,
-        dont_niche_optimize_enum: bool,
     ) -> LayoutCalculatorResult<FieldIdx, VariantIdx, F> {
         // Until we've decided whether to use the tagged or
         // niche filling LayoutData, we don't want to intern the
@@ -618,7 +610,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
         }
 
         let calculate_niche_filling_layout = || -> Option<TmpLayout<FieldIdx, VariantIdx>> {
-            if dont_niche_optimize_enum {
+            if repr.inhibit_enum_layout_opt() {
                 return None;
             }
 

compiler/rustc_abi/src/lib.rs

@@ -1376,6 +1376,28 @@ impl WrappingRange {
         }
     }
 
+    /// Returns `true` if all the values in `other` are contained in this range,
+    /// when the values are considered as having width `size`.
+    #[inline(always)]
+    pub fn contains_range(&self, other: Self, size: Size) -> bool {
+        if self.is_full_for(size) {
+            true
+        } else {
+            let trunc = |x| size.truncate(x);
+
+            let delta = self.start;
+            let max = trunc(self.end.wrapping_sub(delta));
+
+            let other_start = trunc(other.start.wrapping_sub(delta));
+            let other_end = trunc(other.end.wrapping_sub(delta));
+
+            // Having shifted both input ranges by `delta`, now we only need to check
+            // whether `0..=max` contains `other_start..=other_end`, which can only
+            // happen if the other doesn't wrap since `self` isn't everything.
+            (other_start <= other_end) && (other_end <= max)
+        }
+    }
+
     /// Returns `self` with replaced `start`
     #[inline(always)]
     fn with_start(mut self, start: u128) -> Self {

compiler/rustc_abi/src/tests.rs

@@ -5,3 +5,66 @@ fn align_constants() {
     assert_eq!(Align::ONE, Align::from_bytes(1).unwrap());
     assert_eq!(Align::EIGHT, Align::from_bytes(8).unwrap());
 }
+
+#[test]
+fn wrapping_range_contains_range() {
+    let size16 = Size::from_bytes(16);
+
+    let a = WrappingRange { start: 10, end: 20 };
+    assert!(a.contains_range(a, size16));
+    assert!(a.contains_range(WrappingRange { start: 11, end: 19 }, size16));
+    assert!(a.contains_range(WrappingRange { start: 10, end: 10 }, size16));
+    assert!(a.contains_range(WrappingRange { start: 20, end: 20 }, size16));
+    assert!(!a.contains_range(WrappingRange { start: 10, end: 21 }, size16));
+    assert!(!a.contains_range(WrappingRange { start: 9, end: 20 }, size16));
+    assert!(!a.contains_range(WrappingRange { start: 4, end: 6 }, size16));
+    assert!(!a.contains_range(WrappingRange { start: 24, end: 26 }, size16));
+
+    assert!(!a.contains_range(WrappingRange { start: 16, end: 14 }, size16));
+
+    let b = WrappingRange { start: 20, end: 10 };
+    assert!(b.contains_range(b, size16));
+    assert!(b.contains_range(WrappingRange { start: 20, end: 20 }, size16));
+    assert!(b.contains_range(WrappingRange { start: 10, end: 10 }, size16));
+    assert!(b.contains_range(WrappingRange { start: 0, end: 10 }, size16));
+    assert!(b.contains_range(WrappingRange { start: 20, end: 30 }, size16));
+    assert!(b.contains_range(WrappingRange { start: 20, end: 9 }, size16));
+    assert!(b.contains_range(WrappingRange { start: 21, end: 10 }, size16));
+    assert!(b.contains_range(WrappingRange { start: 999, end: 9999 }, size16));
+    assert!(b.contains_range(WrappingRange { start: 999, end: 9 }, size16));
+    assert!(!b.contains_range(WrappingRange { start: 19, end: 19 }, size16));
+    assert!(!b.contains_range(WrappingRange { start: 11, end: 11 }, size16));
+    assert!(!b.contains_range(WrappingRange { start: 19, end: 11 }, size16));
+    assert!(!b.contains_range(WrappingRange { start: 11, end: 19 }, size16));
+
+    let f = WrappingRange { start: 0, end: u128::MAX };
+    assert!(f.contains_range(WrappingRange { start: 10, end: 20 }, size16));
+    assert!(f.contains_range(WrappingRange { start: 20, end: 10 }, size16));
+
+    let g = WrappingRange { start: 2, end: 1 };
+    assert!(g.contains_range(WrappingRange { start: 10, end: 20 }, size16));
+    assert!(g.contains_range(WrappingRange { start: 20, end: 10 }, size16));
+
+    let size1 = Size::from_bytes(1);
+    let u8r = WrappingRange { start: 0, end: 255 };
+    let i8r = WrappingRange { start: 128, end: 127 };
+    assert!(u8r.contains_range(i8r, size1));
+    assert!(i8r.contains_range(u8r, size1));
+    assert!(!u8r.contains_range(i8r, size16));
+    assert!(i8r.contains_range(u8r, size16));
+
+    let boolr = WrappingRange { start: 0, end: 1 };
+    assert!(u8r.contains_range(boolr, size1));
+    assert!(i8r.contains_range(boolr, size1));
+    assert!(!boolr.contains_range(u8r, size1));
+    assert!(!boolr.contains_range(i8r, size1));
+
+    let cmpr = WrappingRange { start: 255, end: 1 };
+    assert!(u8r.contains_range(cmpr, size1));
+    assert!(i8r.contains_range(cmpr, size1));
+    assert!(!cmpr.contains_range(u8r, size1));
+    assert!(!cmpr.contains_range(i8r, size1));
+
+    assert!(!boolr.contains_range(cmpr, size1));
+    assert!(cmpr.contains_range(boolr, size1));
+}

compiler/rustc_borrowck/src/diagnostics/move_errors.rs

@@ -115,10 +115,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
     fn append_to_grouped_errors(
         &self,
         grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
-        error: MoveError<'tcx>,
+        MoveError { place: original_path, location, kind }: MoveError<'tcx>,
     ) {
-        let MoveError { place: original_path, location, kind } = error;
-
         // Note: that the only time we assign a place isn't a temporary
         // to a user variable is when initializing it.
         // If that ever stops being the case, then the ever initialized
@@ -251,54 +249,47 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
     }
 
     fn report(&mut self, error: GroupedMoveError<'tcx>) {
-        let (mut err, err_span) = {
-            let (span, use_spans, original_path, kind) = match error {
-                GroupedMoveError::MovesFromPlace { span, original_path, ref kind, .. }
-                | GroupedMoveError::MovesFromValue { span, original_path, ref kind, .. } => {
-                    (span, None, original_path, kind)
-                }
-                GroupedMoveError::OtherIllegalMove { use_spans, original_path, ref kind } => {
-                    (use_spans.args_or_use(), Some(use_spans), original_path, kind)
-                }
-            };
-            debug!(
-                "report: original_path={:?} span={:?}, kind={:?} \
-                   original_path.is_upvar_field_projection={:?}",
-                original_path,
-                span,
-                kind,
-                self.is_upvar_field_projection(original_path.as_ref())
-            );
-            if self.has_ambiguous_copy(original_path.ty(self.body, self.infcx.tcx).ty) {
-                // If the type may implement Copy, skip the error.
-                // It's an error with the Copy implementation (e.g. duplicate Copy) rather than borrow check
-                self.dcx().span_delayed_bug(
+        let (span, use_spans, original_path, kind) = match error {
+            GroupedMoveError::MovesFromPlace { span, original_path, ref kind, .. }
+            | GroupedMoveError::MovesFromValue { span, original_path, ref kind, .. } => {
+                (span, None, original_path, kind)
+            }
+            GroupedMoveError::OtherIllegalMove { use_spans, original_path, ref kind } => {
+                (use_spans.args_or_use(), Some(use_spans), original_path, kind)
+            }
+        };
+        debug!(
+            "report: original_path={:?} span={:?}, kind={:?} \
+             original_path.is_upvar_field_projection={:?}",
+            original_path,
+            span,
+            kind,
+            self.is_upvar_field_projection(original_path.as_ref())
+        );
+        if self.has_ambiguous_copy(original_path.ty(self.body, self.infcx.tcx).ty) {
+            // If the type may implement Copy, skip the error.
+            // It's an error with the Copy implementation (e.g. duplicate Copy) rather than borrow check
+            self.dcx()
+                .span_delayed_bug(span, "Type may implement copy, but there is no other error.");
+            return;
+        }
+        let mut err = match kind {
+            &IllegalMoveOriginKind::BorrowedContent { target_place } => self
+                .report_cannot_move_from_borrowed_content(
+                    original_path,
+                    target_place,
                     span,
-                    "Type may implement copy, but there is no other error.",
-                );
-                return;
+                    use_spans,
+                ),
+            &IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
+                self.cannot_move_out_of_interior_of_drop(span, ty)
+            }
+            &IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } => {
+                self.cannot_move_out_of_interior_noncopy(span, ty, Some(is_index))
             }
-            (
-                match kind {
-                    &IllegalMoveOriginKind::BorrowedContent { target_place } => self
-                        .report_cannot_move_from_borrowed_content(
-                            original_path,
-                            target_place,
-                            span,
-                            use_spans,
-                        ),
-                    &IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
-                        self.cannot_move_out_of_interior_of_drop(span, ty)
-                    }
-                    &IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } => {
-                        self.cannot_move_out_of_interior_noncopy(span, ty, Some(is_index))
-                    }
-                },
-                span,
-            )
         };
 
-        self.add_move_hints(error, &mut err, err_span);
+        self.add_move_hints(error, &mut err, span);
         self.buffer_error(err);
     }
 
@@ -483,7 +474,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 self.cannot_move_out_of_interior_noncopy(span, ty, None)
             }
             ty::Closure(def_id, closure_args)
-                if def_id.as_local() == Some(self.mir_def_id()) && upvar_field.is_some() =>
+                if def_id.as_local() == Some(self.mir_def_id())
+                    && let Some(upvar_field) = upvar_field =>
             {
                 let closure_kind_ty = closure_args.as_closure().kind_ty();
                 let closure_kind = match closure_kind_ty.to_opt_closure_kind() {
@@ -496,7 +488,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 let capture_description =
                     format!("captured variable in an `{closure_kind}` closure");
 
-                let upvar = &self.upvars[upvar_field.unwrap().index()];
+                let upvar = &self.upvars[upvar_field.index()];
                 let upvar_hir_id = upvar.get_root_variable();
                 let upvar_name = upvar.to_string(tcx);
                 let upvar_span = tcx.hir_span(upvar_hir_id);
@@ -606,7 +598,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             }
             // No binding. Nothing to suggest.
             GroupedMoveError::OtherIllegalMove { ref original_path, use_spans, .. } => {
-                let use_span = use_spans.var_or_use();
+                let mut use_span = use_spans.var_or_use();
                 let place_ty = original_path.ty(self.body, self.infcx.tcx).ty;
                 let place_desc = match self.describe_place(original_path.as_ref()) {
                     Some(desc) => format!("`{desc}`"),
@@ -623,19 +615,59 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                     );
                 }
 
+                if let Some(upvar_field) = self
+                    .prefixes(original_path.as_ref(), PrefixSet::All)
+                    .find_map(|p| self.is_upvar_field_projection(p))
+                {
+                    // Look for the introduction of the original binding being moved.
+                    let upvar = &self.upvars[upvar_field.index()];
+                    let upvar_hir_id = upvar.get_root_variable();
+                    use_span = match self.infcx.tcx.parent_hir_node(upvar_hir_id) {
+                        hir::Node::Param(param) => {
+                            // Instead of pointing at the path where we access the value within a
+                            // closure, we point at the type of the outer `fn` argument.
+                            param.ty_span
+                        }
+                        hir::Node::LetStmt(stmt) => match (stmt.ty, stmt.init) {
+                            // We point at the type of the outer let-binding.
+                            (Some(ty), _) => ty.span,
+                            // We point at the initializer of the outer let-binding, but only if it
+                            // isn't something that spans multiple lines, like a closure, as the
+                            // ASCII art gets messy.
+                            (None, Some(init))
+                                if !self.infcx.tcx.sess.source_map().is_multiline(init.span) =>
+                            {
+                                init.span
+                            }
+                            _ => use_span,
+                        },
+                        _ => use_span,
+                    };
+                }
+
                 err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {
                     is_partial_move: false,
                     ty: place_ty,
                     place: &place_desc,
                     span: use_span,
                 });
 
+                let mut pointed_at_span = false;
                 use_spans.args_subdiag(err, |args_span| {
+                    if args_span == span || args_span == use_span {
+                        pointed_at_span = true;
+                    }
                     crate::session_diagnostics::CaptureArgLabel::MoveOutPlace {
-                        place: place_desc,
+                        place: place_desc.clone(),
                         args_span,
                     }
                 });
+                if !pointed_at_span && use_span != span {
+                    err.subdiagnostic(crate::session_diagnostics::CaptureArgLabel::MoveOutPlace {
+                        place: place_desc,
+                        args_span: span,
+                    });
+                }
 
                 self.add_note_for_packed_struct_derive(err, original_path.local);
             }

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

@@ -288,7 +288,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         // valid ranges. For example, `char`s are passed as just `i32`, with no
         // way for LLVM to know that they're 0x10FFFF at most. Thus we assume
         // the range of the input value too, not just the output range.
-        assume_scalar_range(bx, imm, from_scalar, from_backend_ty);
+        assume_scalar_range(bx, imm, from_scalar, from_backend_ty, None);
 
         imm = match (from_scalar.primitive(), to_scalar.primitive()) {
             (Int(_, is_signed), Int(..)) => bx.intcast(imm, to_backend_ty, is_signed),
@@ -1064,7 +1064,7 @@ pub(super) fn transmute_scalar<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     // That said, last time we tried removing this, it didn't actually help
     // the rustc-perf results, so might as well keep doing it
     // <https://github.com/rust-lang/rust/pull/135610#issuecomment-2599275182>
-    assume_scalar_range(bx, imm, from_scalar, from_backend_ty);
+    assume_scalar_range(bx, imm, from_scalar, from_backend_ty, Some(&to_scalar));
 
     imm = match (from_scalar.primitive(), to_scalar.primitive()) {
         (Int(..) | Float(_), Int(..) | Float(_)) => bx.bitcast(imm, to_backend_ty),
@@ -1092,22 +1092,42 @@ pub(super) fn transmute_scalar<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     // since it's never passed to something with parameter metadata (especially
     // after MIR inlining) so the only way to tell the backend about the
     // constraint that the `transmute` introduced is to `assume` it.
-    assume_scalar_range(bx, imm, to_scalar, to_backend_ty);
+    assume_scalar_range(bx, imm, to_scalar, to_backend_ty, Some(&from_scalar));
 
     imm = bx.to_immediate_scalar(imm, to_scalar);
     imm
 }
 
+/// Emits an `assume` call that `imm`'s value is within the known range of `scalar`.
+///
+/// If `known` is `Some`, only emits the assume if it's more specific than
+/// whatever is already known from the range of *that* scalar.
 fn assume_scalar_range<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     bx: &mut Bx,
     imm: Bx::Value,
     scalar: abi::Scalar,
     backend_ty: Bx::Type,
+    known: Option<&abi::Scalar>,
 ) {
-    if matches!(bx.cx().sess().opts.optimize, OptLevel::No) || scalar.is_always_valid(bx.cx()) {
+    if matches!(bx.cx().sess().opts.optimize, OptLevel::No) {
         return;
     }
 
+    match (scalar, known) {
+        (abi::Scalar::Union { .. }, _) => return,
+        (_, None) => {
+            if scalar.is_always_valid(bx.cx()) {
+                return;
+            }
+        }
+        (abi::Scalar::Initialized { valid_range, .. }, Some(known)) => {
+            let known_range = known.valid_range(bx.cx());
+            if valid_range.contains_range(known_range, scalar.size(bx.cx())) {
+                return;
+            }
+        }
+    }
+
     match scalar.primitive() {
         abi::Primitive::Int(..) => {
             let range = scalar.valid_range(bx.cx());