Skip to content

Commit 02cd79a

Browse files
committed
Auto merge of #102652 - Dylan-DPC:rollup-6ff8ct8, r=Dylan-DPC
Rollup of 6 pull requests Successful merges: - #101189 (Implement `Ready::into_inner()`) - #101642 (Fix in-place collection leak when remaining element destructor panic) - #102489 (Normalize substs before resolving instance in `NoopMethodCall` lint) - #102559 (Don't ICE when trying to copy unsized value in const prop) - #102568 (Lint against nested opaque types that don't satisfy associated type bounds) - #102633 (Fix rustdoc ICE in invalid_rust_codeblocks lint) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
2 parents ead49f0 + f7ca465 commit 02cd79a

31 files changed

+467
-58
lines changed

compiler/rustc_const_eval/src/interpret/place.rs

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

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

+4
Original file line numberDiff line numberDiff line change
@@ -433,3 +433,7 @@ lint_check_name_unknown_tool = unknown lint tool: `{$tool_name}`
433433
lint_check_name_warning = {$msg}
434434
435435
lint_check_name_deprecated = lint name `{$lint_name}` is deprecated and does not have an effect anymore. Use: {$new_name}
436+
437+
lint_opaque_hidden_inferred_bound = opaque type `{$ty}` does not satisfy its associated type bounds
438+
.specifically = this associated type bound is unsatisfied for `{$proj_ty}`
439+
.suggestion = add this bound

compiler/rustc_lint/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ mod non_ascii_idents;
6262
mod non_fmt_panic;
6363
mod nonstandard_style;
6464
mod noop_method_call;
65+
mod opaque_hidden_inferred_bound;
6566
mod pass_by_value;
6667
mod passes;
6768
mod redundant_semicolon;
@@ -93,6 +94,7 @@ use non_ascii_idents::*;
9394
use non_fmt_panic::NonPanicFmt;
9495
use nonstandard_style::*;
9596
use noop_method_call::*;
97+
use opaque_hidden_inferred_bound::*;
9698
use pass_by_value::*;
9799
use redundant_semicolon::*;
98100
use traits::*;
@@ -223,6 +225,7 @@ macro_rules! late_lint_mod_passes {
223225
EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums,
224226
InvalidAtomicOrdering: InvalidAtomicOrdering,
225227
NamedAsmLabels: NamedAsmLabels,
228+
OpaqueHiddenInferredBound: OpaqueHiddenInferredBound,
226229
]
227230
);
228231
};

compiler/rustc_lint/src/noop_method_call.rs

+6-11
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use crate::context::LintContext;
2-
use crate::rustc_middle::ty::TypeVisitable;
32
use crate::LateContext;
43
use crate::LateLintPass;
54
use rustc_errors::fluent;
@@ -46,7 +45,7 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
4645
};
4746
// We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow`
4847
// traits and ignore any other method call.
49-
let (trait_id, did) = match cx.typeck_results().type_dependent_def(expr.hir_id) {
48+
let did = match cx.typeck_results().type_dependent_def(expr.hir_id) {
5049
// Verify we are dealing with a method/associated function.
5150
Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) {
5251
// 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 {
5655
Some(sym::Borrow | sym::Clone | sym::Deref)
5756
) =>
5857
{
59-
(trait_id, did)
58+
did
6059
}
6160
_ => return,
6261
},
6362
_ => return,
6463
};
65-
let substs = cx.typeck_results().node_substs(expr.hir_id);
66-
if substs.needs_subst() {
67-
// We can't resolve on types that require monomorphization, so we don't handle them if
68-
// we need to perform substitution.
69-
return;
70-
}
71-
let param_env = cx.tcx.param_env(trait_id);
64+
let substs = cx
65+
.tcx
66+
.normalize_erasing_regions(cx.param_env, cx.typeck_results().node_substs(expr.hir_id));
7267
// Resolve the trait method instance.
73-
let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) else {
68+
let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, cx.param_env, did, substs) else {
7469
return
7570
};
7671
// (Re)check that it implements the noop diagnostic.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
use rustc_hir as hir;
2+
use rustc_infer::infer::TyCtxtInferExt;
3+
use rustc_macros::LintDiagnostic;
4+
use rustc_middle::ty::{self, fold::BottomUpFolder, Ty, TypeFoldable};
5+
use rustc_span::Span;
6+
use rustc_trait_selection::traits;
7+
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
8+
9+
use crate::{LateContext, LateLintPass, LintContext};
10+
11+
declare_lint! {
12+
/// The `opaque_hidden_inferred_bound` lint detects cases in which nested
13+
/// `impl Trait` in associated type bounds are not written generally enough
14+
/// to satisfy the bounds of the associated type.
15+
///
16+
/// ### Explanation
17+
///
18+
/// This functionality was removed in #97346, but then rolled back in #99860
19+
/// because it caused regressions.
20+
///
21+
/// We plan on reintroducing this as a hard error, but in the mean time,
22+
/// this lint serves to warn and suggest fixes for any use-cases which rely
23+
/// on this behavior.
24+
///
25+
/// ### Example
26+
///
27+
/// ```
28+
/// trait Trait {
29+
/// type Assoc: Send;
30+
/// }
31+
///
32+
/// struct Struct;
33+
///
34+
/// impl Trait for Struct {
35+
/// type Assoc = i32;
36+
/// }
37+
///
38+
/// fn test() -> impl Trait<Assoc = impl Sized> {
39+
/// Struct
40+
/// }
41+
/// ```
42+
///
43+
/// {{produces}}
44+
///
45+
/// In this example, `test` declares that the associated type `Assoc` for
46+
/// `impl Trait` is `impl Sized`, which does not satisfy the `Send` bound
47+
/// on the associated type.
48+
///
49+
/// Although the hidden type, `i32` does satisfy this bound, we do not
50+
/// consider the return type to be well-formed with this lint. It can be
51+
/// fixed by changing `impl Sized` into `impl Sized + Send`.
52+
pub OPAQUE_HIDDEN_INFERRED_BOUND,
53+
Warn,
54+
"detects the use of nested `impl Trait` types in associated type bounds that are not general enough"
55+
}
56+
57+
declare_lint_pass!(OpaqueHiddenInferredBound => [OPAQUE_HIDDEN_INFERRED_BOUND]);
58+
59+
impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound {
60+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
61+
let hir::ItemKind::OpaqueTy(_) = &item.kind else { return; };
62+
let def_id = item.def_id.def_id.to_def_id();
63+
cx.tcx.infer_ctxt().enter(|ref infcx| {
64+
// For every projection predicate in the opaque type's explicit bounds,
65+
// check that the type that we're assigning actually satisfies the bounds
66+
// of the associated type.
67+
for &(pred, pred_span) in cx.tcx.explicit_item_bounds(def_id) {
68+
// Liberate bound regions in the predicate since we
69+
// don't actually care about lifetimes in this check.
70+
let predicate = cx.tcx.liberate_late_bound_regions(
71+
def_id,
72+
pred.kind(),
73+
);
74+
let ty::PredicateKind::Projection(proj) = predicate else {
75+
continue;
76+
};
77+
// Only check types, since those are the only things that may
78+
// have opaques in them anyways.
79+
let Some(proj_term) = proj.term.ty() else { continue };
80+
81+
let proj_ty =
82+
cx
83+
.tcx
84+
.mk_projection(proj.projection_ty.item_def_id, proj.projection_ty.substs);
85+
// For every instance of the projection type in the bounds,
86+
// replace them with the term we're assigning to the associated
87+
// type in our opaque type.
88+
let proj_replacer = &mut BottomUpFolder {
89+
tcx: cx.tcx,
90+
ty_op: |ty| if ty == proj_ty { proj_term } else { ty },
91+
lt_op: |lt| lt,
92+
ct_op: |ct| ct,
93+
};
94+
// For example, in `impl Trait<Assoc = impl Send>`, for all of the bounds on `Assoc`,
95+
// e.g. `type Assoc: OtherTrait`, replace `<impl Trait as Trait>::Assoc: OtherTrait`
96+
// with `impl Send: OtherTrait`.
97+
for assoc_pred_and_span in cx
98+
.tcx
99+
.bound_explicit_item_bounds(proj.projection_ty.item_def_id)
100+
.transpose_iter()
101+
{
102+
let assoc_pred_span = assoc_pred_and_span.0.1;
103+
let assoc_pred = assoc_pred_and_span
104+
.map_bound(|(pred, _)| *pred)
105+
.subst(cx.tcx, &proj.projection_ty.substs)
106+
.fold_with(proj_replacer);
107+
let Ok(assoc_pred) = traits::fully_normalize(infcx, traits::ObligationCause::dummy(), cx.param_env, assoc_pred) else {
108+
continue;
109+
};
110+
// If that predicate doesn't hold modulo regions (but passed during type-check),
111+
// then we must've taken advantage of the hack in `project_and_unify_types` where
112+
// we replace opaques with inference vars. Emit a warning!
113+
if !infcx.predicate_must_hold_modulo_regions(&traits::Obligation::new(
114+
traits::ObligationCause::dummy(),
115+
cx.param_env,
116+
assoc_pred,
117+
)) {
118+
// If it's a trait bound and an opaque that doesn't satisfy it,
119+
// then we can emit a suggestion to add the bound.
120+
let (suggestion, suggest_span) =
121+
match (proj_term.kind(), assoc_pred.kind().skip_binder()) {
122+
(ty::Opaque(def_id, _), ty::PredicateKind::Trait(trait_pred)) => (
123+
format!(" + {}", trait_pred.print_modifiers_and_trait_path()),
124+
Some(cx.tcx.def_span(def_id).shrink_to_hi()),
125+
),
126+
_ => (String::new(), None),
127+
};
128+
cx.emit_spanned_lint(
129+
OPAQUE_HIDDEN_INFERRED_BOUND,
130+
pred_span,
131+
OpaqueHiddenInferredBoundLint {
132+
ty: cx.tcx.mk_opaque(def_id, ty::InternalSubsts::identity_for_item(cx.tcx, def_id)),
133+
proj_ty: proj_term,
134+
assoc_pred_span,
135+
suggestion,
136+
suggest_span,
137+
},
138+
);
139+
}
140+
}
141+
}
142+
});
143+
}
144+
}
145+
146+
#[derive(LintDiagnostic)]
147+
#[diag(lint::opaque_hidden_inferred_bound)]
148+
struct OpaqueHiddenInferredBoundLint<'tcx> {
149+
ty: Ty<'tcx>,
150+
proj_ty: Ty<'tcx>,
151+
#[label(lint::specifically)]
152+
assoc_pred_span: Span,
153+
#[suggestion_verbose(applicability = "machine-applicable", code = "{suggestion}")]
154+
suggest_span: Option<Span>,
155+
suggestion: String,
156+
}

compiler/rustc_middle/src/lint.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,9 @@ pub fn in_external_macro(sess: &Session, span: Span) -> bool {
446446
match expn_data.kind {
447447
ExpnKind::Inlined
448448
| ExpnKind::Root
449-
| ExpnKind::Desugaring(DesugaringKind::ForLoop | DesugaringKind::WhileLoop) => false,
449+
| ExpnKind::Desugaring(
450+
DesugaringKind::ForLoop | DesugaringKind::WhileLoop | DesugaringKind::OpaqueTy,
451+
) => false,
450452
ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) => true, // well, it's "external"
451453
ExpnKind::Macro(MacroKind::Bang, _) => {
452454
// Dummy span for the `def_site` means it's an external macro.

library/alloc/src/vec/in_place_collect.rs

+10-4
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@
5555
//! This is handled by the [`InPlaceDrop`] guard for sink items (`U`) and by
5656
//! [`vec::IntoIter::forget_allocation_drop_remaining()`] for remaining source items (`T`).
5757
//!
58+
//! If dropping any remaining source item (`T`) panics then [`InPlaceDstBufDrop`] will handle dropping
59+
//! the already collected sink items (`U`) and freeing the allocation.
60+
//!
5861
//! [`vec::IntoIter::forget_allocation_drop_remaining()`]: super::IntoIter::forget_allocation_drop_remaining()
5962
//!
6063
//! # O(1) collect
@@ -138,7 +141,7 @@ use core::iter::{InPlaceIterable, SourceIter, TrustedRandomAccessNoCoerce};
138141
use core::mem::{self, ManuallyDrop, SizedTypeProperties};
139142
use core::ptr::{self};
140143

141-
use super::{InPlaceDrop, SpecFromIter, SpecFromIterNested, Vec};
144+
use super::{InPlaceDrop, InPlaceDstBufDrop, SpecFromIter, SpecFromIterNested, Vec};
142145

143146
/// Specialization marker for collecting an iterator pipeline into a Vec while reusing the
144147
/// source allocation, i.e. executing the pipeline in place.
@@ -191,14 +194,17 @@ where
191194
);
192195
}
193196

194-
// Drop any remaining values at the tail of the source but prevent drop of the allocation
195-
// itself once IntoIter goes out of scope.
196-
// If the drop panics then we also leak any elements collected into dst_buf.
197+
// The ownership of the allocation and the new `T` values is temporarily moved into `dst_guard`.
198+
// This is safe because `forget_allocation_drop_remaining` immediately forgets the allocation
199+
// before any panic can occur in order to avoid any double free, and then proceeds to drop
200+
// any remaining values at the tail of the source.
197201
//
198202
// Note: This access to the source wouldn't be allowed by the TrustedRandomIteratorNoCoerce
199203
// contract (used by SpecInPlaceCollect below). But see the "O(1) collect" section in the
200204
// module documenttation why this is ok anyway.
205+
let dst_guard = InPlaceDstBufDrop { ptr: dst_buf, len, cap };
201206
src.forget_allocation_drop_remaining();
207+
mem::forget(dst_guard);
202208

203209
let vec = unsafe { Vec::from_raw_parts(dst_buf, len, cap) };
204210

library/alloc/src/vec/in_place_drop.rs

+15
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,18 @@ impl<T> Drop for InPlaceDrop<T> {
2222
}
2323
}
2424
}
25+
26+
// A helper struct for in-place collection that drops the destination allocation and elements,
27+
// to avoid leaking them if some other destructor panics.
28+
pub(super) struct InPlaceDstBufDrop<T> {
29+
pub(super) ptr: *mut T,
30+
pub(super) len: usize,
31+
pub(super) cap: usize,
32+
}
33+
34+
impl<T> Drop for InPlaceDstBufDrop<T> {
35+
#[inline]
36+
fn drop(&mut self) {
37+
unsafe { super::Vec::from_raw_parts(self.ptr, self.len, self.cap) };
38+
}
39+
}

library/alloc/src/vec/into_iter.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,16 @@ impl<T, A: Allocator> IntoIter<T, A> {
9595
}
9696

9797
/// Drops remaining elements and relinquishes the backing allocation.
98+
/// This method guarantees it won't panic before relinquishing
99+
/// the backing allocation.
98100
///
99101
/// This is roughly equivalent to the following, but more efficient
100102
///
101103
/// ```
102104
/// # let mut into_iter = Vec::<u8>::with_capacity(10).into_iter();
105+
/// let mut into_iter = std::mem::replace(&mut into_iter, Vec::new().into_iter());
103106
/// (&mut into_iter).for_each(core::mem::drop);
104-
/// unsafe { core::ptr::write(&mut into_iter, Vec::new().into_iter()); }
107+
/// std::mem::forget(into_iter);
105108
/// ```
106109
///
107110
/// 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> {
118121
self.ptr = self.buf.as_ptr();
119122
self.end = self.buf.as_ptr();
120123

124+
// Dropping the remaining elements can panic, so this needs to be
125+
// done only after updating the other fields.
121126
unsafe {
122127
ptr::drop_in_place(remaining);
123128
}

library/alloc/src/vec/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ use self::set_len_on_drop::SetLenOnDrop;
125125
mod set_len_on_drop;
126126

127127
#[cfg(not(no_global_oom_handling))]
128-
use self::in_place_drop::InPlaceDrop;
128+
use self::in_place_drop::{InPlaceDrop, InPlaceDstBufDrop};
129129

130130
#[cfg(not(no_global_oom_handling))]
131131
mod in_place_drop;

0 commit comments

Comments
 (0)