Auto merge of rust-lang#102652 - Dylan-DPC:rollup-6ff8ct8, r=Dylan-DPC

Rollup of 6 pull requests

Successful merges:

 - rust-lang#101189 (Implement `Ready::into_inner()`)
 - rust-lang#101642 (Fix in-place collection leak when remaining element destructor panic)
 - rust-lang#102489 (Normalize substs before resolving instance in `NoopMethodCall` lint)
 - rust-lang#102559 (Don't ICE when trying to copy unsized value in const prop)
 - rust-lang#102568 (Lint against nested opaque types that don't satisfy associated type bounds)
 - rust-lang#102633 (Fix rustdoc ICE in invalid_rust_codeblocks lint)

Failed merges:

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

compiler/rustc_const_eval/src/interpret/place.rs

@@ -640,11 +640,17 @@ where
         // avoid force_allocation.
         let src = match self.read_immediate_raw(src)? {
             Ok(src_val) => {
-                assert!(!src.layout.is_unsized(), "cannot copy unsized immediates");
-                assert!(
-                    !dest.layout.is_unsized(),
-                    "the src is sized, so the dest must also be sized"
-                );
+                // FIXME(const_prop): Const-prop can possibly evaluate an
+                // unsized copy operation when it thinks that the type is
+                // actually sized, due to a trivially false where-clause
+                // predicate like `where Self: Sized` with `Self = dyn Trait`.
+                // See #102553 for an example of such a predicate.
+                if src.layout.is_unsized() {
+                    throw_inval!(SizeOfUnsizedType(src.layout.ty));
+                }
+                if dest.layout.is_unsized() {
+                    throw_inval!(SizeOfUnsizedType(dest.layout.ty));
+                }
                 assert_eq!(src.layout.size, dest.layout.size);
                 // Yay, we got a value that we can write directly.
                 return if layout_compat {

compiler/rustc_error_messages/locales/en-US/lint.ftl

@@ -433,3 +433,7 @@ lint_check_name_unknown_tool = unknown lint tool: `{$tool_name}`
 lint_check_name_warning = {$msg}
 
 lint_check_name_deprecated = lint name `{$lint_name}` is deprecated and does not have an effect anymore. Use: {$new_name}
+
+lint_opaque_hidden_inferred_bound = opaque type `{$ty}` does not satisfy its associated type bounds
+    .specifically = this associated type bound is unsatisfied for `{$proj_ty}`
+    .suggestion = add this bound

compiler/rustc_lint/src/lib.rs

@@ -62,6 +62,7 @@ mod non_ascii_idents;
 mod non_fmt_panic;
 mod nonstandard_style;
 mod noop_method_call;
+mod opaque_hidden_inferred_bound;
 mod pass_by_value;
 mod passes;
 mod redundant_semicolon;
@@ -93,6 +94,7 @@ use non_ascii_idents::*;
 use non_fmt_panic::NonPanicFmt;
 use nonstandard_style::*;
 use noop_method_call::*;
+use opaque_hidden_inferred_bound::*;
 use pass_by_value::*;
 use redundant_semicolon::*;
 use traits::*;
@@ -223,6 +225,7 @@ macro_rules! late_lint_mod_passes {
                 EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums,
                 InvalidAtomicOrdering: InvalidAtomicOrdering,
                 NamedAsmLabels: NamedAsmLabels,
+                OpaqueHiddenInferredBound: OpaqueHiddenInferredBound,
             ]
         );
     };

compiler/rustc_lint/src/noop_method_call.rs

@@ -1,5 +1,4 @@
 use crate::context::LintContext;
-use crate::rustc_middle::ty::TypeVisitable;
 use crate::LateContext;
 use crate::LateLintPass;
 use rustc_errors::fluent;
@@ -46,7 +45,7 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
         };
         // We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow`
         // traits and ignore any other method call.
-        let (trait_id, did) = match cx.typeck_results().type_dependent_def(expr.hir_id) {
+        let did = match cx.typeck_results().type_dependent_def(expr.hir_id) {
             // Verify we are dealing with a method/associated function.
             Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) {
                 // Check that we're dealing with a trait method for one of the traits we care about.
@@ -56,21 +55,17 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
                         Some(sym::Borrow | sym::Clone | sym::Deref)
                     ) =>
                 {
-                    (trait_id, did)
+                    did
                 }
                 _ => return,
             },
             _ => return,
         };
-        let substs = cx.typeck_results().node_substs(expr.hir_id);
-        if substs.needs_subst() {
-            // We can't resolve on types that require monomorphization, so we don't handle them if
-            // we need to perform substitution.
-            return;
-        }
-        let param_env = cx.tcx.param_env(trait_id);
+        let substs = cx
+            .tcx
+            .normalize_erasing_regions(cx.param_env, cx.typeck_results().node_substs(expr.hir_id));
         // Resolve the trait method instance.
-        let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) else {
+        let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, cx.param_env, did, substs) else {
             return
         };
         // (Re)check that it implements the noop diagnostic.

compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs

@@ -0,0 +1,156 @@
+use rustc_hir as hir;
+use rustc_infer::infer::TyCtxtInferExt;
+use rustc_macros::LintDiagnostic;
+use rustc_middle::ty::{self, fold::BottomUpFolder, Ty, TypeFoldable};
+use rustc_span::Span;
+use rustc_trait_selection::traits;
+use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
+
+use crate::{LateContext, LateLintPass, LintContext};
+
+declare_lint! {
+    /// The `opaque_hidden_inferred_bound` lint detects cases in which nested
+    /// `impl Trait` in associated type bounds are not written generally enough
+    /// to satisfy the bounds of the associated type.
+    ///
+    /// ### Explanation
+    ///
+    /// This functionality was removed in #97346, but then rolled back in #99860
+    /// because it caused regressions.
+    ///
+    /// We plan on reintroducing this as a hard error, but in the mean time,
+    /// this lint serves to warn and suggest fixes for any use-cases which rely
+    /// on this behavior.
+    ///
+    /// ### Example
+    ///
+    /// ```
+    /// trait Trait {
+    ///     type Assoc: Send;
+    /// }
+    ///
+    /// struct Struct;
+    ///
+    /// impl Trait for Struct {
+    ///     type Assoc = i32;
+    /// }
+    ///
+    /// fn test() -> impl Trait<Assoc = impl Sized> {
+    ///     Struct
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// In this example, `test` declares that the associated type `Assoc` for
+    /// `impl Trait` is `impl Sized`, which does not satisfy the `Send` bound
+    /// on the associated type.
+    ///
+    /// Although the hidden type, `i32` does satisfy this bound, we do not
+    /// consider the return type to be well-formed with this lint. It can be
+    /// fixed by changing `impl Sized` into `impl Sized + Send`.
+    pub OPAQUE_HIDDEN_INFERRED_BOUND,
+    Warn,
+    "detects the use of nested `impl Trait` types in associated type bounds that are not general enough"
+}
+
+declare_lint_pass!(OpaqueHiddenInferredBound => [OPAQUE_HIDDEN_INFERRED_BOUND]);
+
+impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
+        let hir::ItemKind::OpaqueTy(_) = &item.kind else { return; };
+        let def_id = item.def_id.def_id.to_def_id();
+        cx.tcx.infer_ctxt().enter(|ref infcx| {
+            // For every projection predicate in the opaque type's explicit bounds,
+            // check that the type that we're assigning actually satisfies the bounds
+            // of the associated type.
+            for &(pred, pred_span) in cx.tcx.explicit_item_bounds(def_id) {
+                // Liberate bound regions in the predicate since we
+                // don't actually care about lifetimes in this check.
+                let predicate = cx.tcx.liberate_late_bound_regions(
+                    def_id,
+                    pred.kind(),
+                );
+                let ty::PredicateKind::Projection(proj) = predicate else {
+                    continue;
+                };
+                // Only check types, since those are the only things that may
+                // have opaques in them anyways.
+                let Some(proj_term) = proj.term.ty() else { continue };
+
+                let proj_ty =
+                    cx
+                    .tcx
+                    .mk_projection(proj.projection_ty.item_def_id, proj.projection_ty.substs);
+                // For every instance of the projection type in the bounds,
+                // replace them with the term we're assigning to the associated
+                // type in our opaque type.
+                let proj_replacer = &mut BottomUpFolder {
+                    tcx: cx.tcx,
+                    ty_op: |ty| if ty == proj_ty { proj_term } else { ty },
+                    lt_op: |lt| lt,
+                    ct_op: |ct| ct,
+                };
+                // For example, in `impl Trait<Assoc = impl Send>`, for all of the bounds on `Assoc`,
+                // e.g. `type Assoc: OtherTrait`, replace `<impl Trait as Trait>::Assoc: OtherTrait`
+                // with `impl Send: OtherTrait`.
+                for assoc_pred_and_span in cx
+                    .tcx
+                    .bound_explicit_item_bounds(proj.projection_ty.item_def_id)
+                    .transpose_iter()
+                {
+                    let assoc_pred_span = assoc_pred_and_span.0.1;
+                    let assoc_pred = assoc_pred_and_span
+                        .map_bound(|(pred, _)| *pred)
+                        .subst(cx.tcx, &proj.projection_ty.substs)
+                        .fold_with(proj_replacer);
+                    let Ok(assoc_pred) = traits::fully_normalize(infcx, traits::ObligationCause::dummy(), cx.param_env, assoc_pred) else {
+                        continue;
+                    };
+                    // If that predicate doesn't hold modulo regions (but passed during type-check),
+                    // then we must've taken advantage of the hack in `project_and_unify_types` where
+                    // we replace opaques with inference vars. Emit a warning!
+                    if !infcx.predicate_must_hold_modulo_regions(&traits::Obligation::new(
+                        traits::ObligationCause::dummy(),
+                        cx.param_env,
+                        assoc_pred,
+                    )) {
+                        // If it's a trait bound and an opaque that doesn't satisfy it,
+                        // then we can emit a suggestion to add the bound.
+                        let (suggestion, suggest_span) =
+                            match (proj_term.kind(), assoc_pred.kind().skip_binder()) {
+                                (ty::Opaque(def_id, _), ty::PredicateKind::Trait(trait_pred)) => (
+                                    format!(" + {}", trait_pred.print_modifiers_and_trait_path()),
+                                    Some(cx.tcx.def_span(def_id).shrink_to_hi()),
+                                ),
+                                _ => (String::new(), None),
+                            };
+                        cx.emit_spanned_lint(
+                            OPAQUE_HIDDEN_INFERRED_BOUND,
+                            pred_span,
+                            OpaqueHiddenInferredBoundLint {
+                                ty: cx.tcx.mk_opaque(def_id, ty::InternalSubsts::identity_for_item(cx.tcx, def_id)),
+                                proj_ty: proj_term,
+                                assoc_pred_span,
+                                suggestion,
+                                suggest_span,
+                            },
+                        );
+                    }
+                }
+            }
+        });
+    }
+}
+
+#[derive(LintDiagnostic)]
+#[diag(lint::opaque_hidden_inferred_bound)]
+struct OpaqueHiddenInferredBoundLint<'tcx> {
+    ty: Ty<'tcx>,
+    proj_ty: Ty<'tcx>,
+    #[label(lint::specifically)]
+    assoc_pred_span: Span,
+    #[suggestion_verbose(applicability = "machine-applicable", code = "{suggestion}")]
+    suggest_span: Option<Span>,
+    suggestion: String,
+}

compiler/rustc_middle/src/lint.rs

@@ -446,7 +446,9 @@ pub fn in_external_macro(sess: &Session, span: Span) -> bool {
     match expn_data.kind {
         ExpnKind::Inlined
         | ExpnKind::Root
-        | ExpnKind::Desugaring(DesugaringKind::ForLoop | DesugaringKind::WhileLoop) => false,
+        | ExpnKind::Desugaring(
+            DesugaringKind::ForLoop | DesugaringKind::WhileLoop | DesugaringKind::OpaqueTy,
+        ) => false,
         ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) => true, // well, it's "external"
         ExpnKind::Macro(MacroKind::Bang, _) => {
             // Dummy span for the `def_site` means it's an external macro.

library/alloc/src/vec/in_place_collect.rs

@@ -55,6 +55,9 @@
 //! This is handled by the [`InPlaceDrop`] guard for sink items (`U`) and by
 //! [`vec::IntoIter::forget_allocation_drop_remaining()`] for remaining source items (`T`).
 //!
+//! If dropping any remaining source item (`T`) panics then [`InPlaceDstBufDrop`] will handle dropping
+//! the already collected sink items (`U`) and freeing the allocation.
+//!
 //! [`vec::IntoIter::forget_allocation_drop_remaining()`]: super::IntoIter::forget_allocation_drop_remaining()
 //!
 //! # O(1) collect
@@ -138,7 +141,7 @@ use core::iter::{InPlaceIterable, SourceIter, TrustedRandomAccessNoCoerce};
 use core::mem::{self, ManuallyDrop, SizedTypeProperties};
 use core::ptr::{self};
 
-use super::{InPlaceDrop, SpecFromIter, SpecFromIterNested, Vec};
+use super::{InPlaceDrop, InPlaceDstBufDrop, SpecFromIter, SpecFromIterNested, Vec};
 
 /// Specialization marker for collecting an iterator pipeline into a Vec while reusing the
 /// source allocation, i.e. executing the pipeline in place.
@@ -191,14 +194,17 @@ where
             );
         }
 
-        // Drop any remaining values at the tail of the source but prevent drop of the allocation
-        // itself once IntoIter goes out of scope.
-        // If the drop panics then we also leak any elements collected into dst_buf.
+        // The ownership of the allocation and the new `T` values is temporarily moved into `dst_guard`.
+        // This is safe because `forget_allocation_drop_remaining` immediately forgets the allocation
+        // before any panic can occur in order to avoid any double free, and then proceeds to drop
+        // any remaining values at the tail of the source.
         //
         // Note: This access to the source wouldn't be allowed by the TrustedRandomIteratorNoCoerce
         // contract (used by SpecInPlaceCollect below). But see the "O(1) collect" section in the
         // module documenttation why this is ok anyway.
+        let dst_guard = InPlaceDstBufDrop { ptr: dst_buf, len, cap };
         src.forget_allocation_drop_remaining();
+        mem::forget(dst_guard);
 
         let vec = unsafe { Vec::from_raw_parts(dst_buf, len, cap) };
 

library/alloc/src/vec/in_place_drop.rs

@@ -22,3 +22,18 @@ impl<T> Drop for InPlaceDrop<T> {
         }
     }
 }
+
+// A helper struct for in-place collection that drops the destination allocation and elements,
+// to avoid leaking them if some other destructor panics.
+pub(super) struct InPlaceDstBufDrop<T> {
+    pub(super) ptr: *mut T,
+    pub(super) len: usize,
+    pub(super) cap: usize,
+}
+
+impl<T> Drop for InPlaceDstBufDrop<T> {
+    #[inline]
+    fn drop(&mut self) {
+        unsafe { super::Vec::from_raw_parts(self.ptr, self.len, self.cap) };
+    }
+}

library/alloc/src/vec/into_iter.rs

@@ -95,13 +95,16 @@ impl<T, A: Allocator> IntoIter<T, A> {
     }
 
     /// Drops remaining elements and relinquishes the backing allocation.
+    /// This method guarantees it won't panic before relinquishing
+    /// the backing allocation.
     ///
     /// This is roughly equivalent to the following, but more efficient
     ///
     /// ```
     /// # let mut into_iter = Vec::<u8>::with_capacity(10).into_iter();
+    /// let mut into_iter = std::mem::replace(&mut into_iter, Vec::new().into_iter());
     /// (&mut into_iter).for_each(core::mem::drop);
-    /// unsafe { core::ptr::write(&mut into_iter, Vec::new().into_iter()); }
+    /// std::mem::forget(into_iter);
     /// ```
     ///
     /// This method is used by in-place iteration, refer to the vec::in_place_collect
@@ -118,6 +121,8 @@ impl<T, A: Allocator> IntoIter<T, A> {
         self.ptr = self.buf.as_ptr();
         self.end = self.buf.as_ptr();
 
+        // Dropping the remaining elements can panic, so this needs to be
+        // done only after updating the other fields.
         unsafe {
             ptr::drop_in_place(remaining);
         }

library/alloc/src/vec/mod.rs

@@ -125,7 +125,7 @@ use self::set_len_on_drop::SetLenOnDrop;
 mod set_len_on_drop;
 
 #[cfg(not(no_global_oom_handling))]
-use self::in_place_drop::InPlaceDrop;
+use self::in_place_drop::{InPlaceDrop, InPlaceDstBufDrop};
 
 #[cfg(not(no_global_oom_handling))]
 mod in_place_drop;