From 5ed178741358db3258d804f332d82e497b7eb11a Mon Sep 17 00:00:00 2001 From: dAxpeDDa Date: Tue, 30 Aug 2022 11:21:07 +0200 Subject: [PATCH 01/13] Implement `Ready::into_inner()` --- library/core/src/future/ready.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/library/core/src/future/ready.rs b/library/core/src/future/ready.rs index 48f20f90a3253..a07b63fb62b90 100644 --- a/library/core/src/future/ready.rs +++ b/library/core/src/future/ready.rs @@ -24,6 +24,30 @@ impl Future for Ready { } } +impl Ready { + /// Consumes the `Ready`, returning the wrapped value. + /// + /// # Panics + /// + /// Will panic if this [`Ready`] was already polled to completion. + /// + /// # Examples + /// + /// ``` + /// #![feature(ready_into_inner)] + /// use std::future; + /// + /// let a = future::ready(1); + /// assert_eq!(a.into_inner(), 1); + /// ``` + #[unstable(feature = "ready_into_inner", issue = "101196")] + #[must_use] + #[inline] + pub fn into_inner(self) -> T { + self.0.expect("Called `into_inner()` on `Ready` after completion") + } +} + /// Creates a future that is immediately ready with a value. /// /// Futures created through this function are functionally similar to those From fa61678a7d471cfc8bf166979674d4574a5d3bec Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 10 Sep 2022 11:26:24 +0200 Subject: [PATCH 02/13] Fix leaking in inplace collection when destructor panics --- library/alloc/src/vec/in_place_collect.rs | 8 ++++++-- library/alloc/src/vec/in_place_drop.rs | 15 +++++++++++++++ library/alloc/src/vec/mod.rs | 2 +- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/vec/in_place_collect.rs b/library/alloc/src/vec/in_place_collect.rs index 55dcb84ad16f9..359f1a9cec251 100644 --- a/library/alloc/src/vec/in_place_collect.rs +++ b/library/alloc/src/vec/in_place_collect.rs @@ -138,7 +138,7 @@ use core::iter::{InPlaceIterable, SourceIter, TrustedRandomAccessNoCoerce}; use core::mem::{self, ManuallyDrop}; 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. @@ -193,12 +193,16 @@ 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. + // If the drop panics then we also try to drop the destination buffer and its elements. + // This is safe because `forget_allocation_drop_remaining` forgets the allocation *before* + // trying to drop the remaining elements. // // 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) }; diff --git a/library/alloc/src/vec/in_place_drop.rs b/library/alloc/src/vec/in_place_drop.rs index 1b1ef9130face..25ca33c6a7bf0 100644 --- a/library/alloc/src/vec/in_place_drop.rs +++ b/library/alloc/src/vec/in_place_drop.rs @@ -22,3 +22,18 @@ impl Drop for InPlaceDrop { } } } + +// 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 { + pub(super) ptr: *mut T, + pub(super) len: usize, + pub(super) cap: usize, +} + +impl Drop for InPlaceDstBufDrop { + #[inline] + fn drop(&mut self) { + unsafe { super::Vec::from_raw_parts(self.ptr, self.len, self.cap) }; + } +} diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index fa9f2131c0c1d..acfbb98272dd5 100644 --- a/library/alloc/src/vec/mod.rs +++ b/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; From f81b07e9470728a311a5d816616762e37b00f29f Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 10 Sep 2022 11:27:21 +0200 Subject: [PATCH 03/13] Adapt inplace collection leak test to check for no leaks --- library/alloc/tests/vec.rs | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index d94da8f5f5a0e..ebf5cbd0e60e0 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -1110,11 +1110,13 @@ fn test_from_iter_specialization_panic_during_iteration_drops() { } #[test] -fn test_from_iter_specialization_panic_during_drop_leaks() { - static mut DROP_COUNTER: usize = 0; +fn test_from_iter_specialization_panic_during_drop_doesnt_leak() { + static mut DROP_COUNTER_SHOULD_BE_DROPPED: usize = 0; + static mut DROP_COUNTER_DROPPED_TWICE: usize = 0; #[derive(Debug)] enum Droppable { + ShouldBeDropped, DroppedTwice(Box), PanicOnDrop, } @@ -1122,11 +1124,17 @@ fn test_from_iter_specialization_panic_during_drop_leaks() { impl Drop for Droppable { fn drop(&mut self) { match self { + Droppable::ShouldBeDropped => { + unsafe { + DROP_COUNTER_SHOULD_BE_DROPPED += 1; + } + println!("Dropping ShouldBeDropped!") + } Droppable::DroppedTwice(_) => { unsafe { - DROP_COUNTER += 1; + DROP_COUNTER_DROPPED_TWICE += 1; } - println!("Dropping!") + println!("Dropping DroppedTwice!") } Droppable::PanicOnDrop => { if !std::thread::panicking() { @@ -1137,21 +1145,17 @@ fn test_from_iter_specialization_panic_during_drop_leaks() { } } - let mut to_free: *mut Droppable = core::ptr::null_mut(); - let mut cap = 0; - let _ = std::panic::catch_unwind(AssertUnwindSafe(|| { - let mut v = vec![Droppable::DroppedTwice(Box::new(123)), Droppable::PanicOnDrop]; - to_free = v.as_mut_ptr(); - cap = v.capacity(); - let _ = v.into_iter().take(0).collect::>(); + let v = vec![ + Droppable::ShouldBeDropped, + Droppable::DroppedTwice(Box::new(123)), + Droppable::PanicOnDrop, + ]; + let _ = v.into_iter().take(1).collect::>(); })); - assert_eq!(unsafe { DROP_COUNTER }, 1); - // clean up the leak to keep miri happy - unsafe { - drop(Vec::from_raw_parts(to_free, 0, cap)); - } + assert_eq!(unsafe { DROP_COUNTER_SHOULD_BE_DROPPED }, 1); + assert_eq!(unsafe { DROP_COUNTER_DROPPED_TWICE }, 1); } // regression test for issue #85322. Peekable previously implemented InPlaceIterable, From f52082f54321e56fa4dbd9194c1cfd61089e2729 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 10 Sep 2022 12:29:16 +0200 Subject: [PATCH 04/13] Update documentation --- library/alloc/src/vec/in_place_collect.rs | 7 +++++-- library/alloc/src/vec/into_iter.rs | 7 ++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/vec/in_place_collect.rs b/library/alloc/src/vec/in_place_collect.rs index 359f1a9cec251..83ed3d915a291 100644 --- a/library/alloc/src/vec/in_place_collect.rs +++ b/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 @@ -194,8 +197,8 @@ 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 try to drop the destination buffer and its elements. - // This is safe because `forget_allocation_drop_remaining` forgets the allocation *before* - // trying to drop the remaining elements. + // This is safe because `forget_allocation_drop_remaining` immediately forgets the allocation + // and won't panic before that. // // 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 diff --git a/library/alloc/src/vec/into_iter.rs b/library/alloc/src/vec/into_iter.rs index 1b483e3fc7793..b5a03392a1797 100644 --- a/library/alloc/src/vec/into_iter.rs +++ b/library/alloc/src/vec/into_iter.rs @@ -96,13 +96,16 @@ impl IntoIter { } /// 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::::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 @@ -119,6 +122,8 @@ impl IntoIter { 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); } From dad049cb5cb3d259836cfe6a9160521d9d4809ca Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 10 Sep 2022 12:29:25 +0200 Subject: [PATCH 05/13] Update test --- library/alloc/tests/vec.rs | 67 +++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index ebf5cbd0e60e0..990ce38e22edd 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -1111,51 +1111,52 @@ fn test_from_iter_specialization_panic_during_iteration_drops() { #[test] fn test_from_iter_specialization_panic_during_drop_doesnt_leak() { - static mut DROP_COUNTER_SHOULD_BE_DROPPED: usize = 0; - static mut DROP_COUNTER_DROPPED_TWICE: usize = 0; + static mut DROP_COUNTER_OLD: [usize; 5] = [0; 5]; + static mut DROP_COUNTER_NEW: [usize; 2] = [0; 2]; #[derive(Debug)] - enum Droppable { - ShouldBeDropped, - DroppedTwice(Box), - PanicOnDrop, + struct Old(usize); + + impl Drop for Old { + fn drop(&mut self) { + unsafe { + DROP_COUNTER_OLD[self.0] += 1; + } + + if self.0 == 3 { + panic!(); + } + + println!("Dropped Old: {}", self.0); + } } - impl Drop for Droppable { + #[derive(Debug)] + struct New(usize); + + impl Drop for New { fn drop(&mut self) { - match self { - Droppable::ShouldBeDropped => { - unsafe { - DROP_COUNTER_SHOULD_BE_DROPPED += 1; - } - println!("Dropping ShouldBeDropped!") - } - Droppable::DroppedTwice(_) => { - unsafe { - DROP_COUNTER_DROPPED_TWICE += 1; - } - println!("Dropping DroppedTwice!") - } - Droppable::PanicOnDrop => { - if !std::thread::panicking() { - panic!(); - } - } + unsafe { + DROP_COUNTER_NEW[self.0] += 1; } + + println!("Dropped New: {}", self.0); } } let _ = std::panic::catch_unwind(AssertUnwindSafe(|| { - let v = vec![ - Droppable::ShouldBeDropped, - Droppable::DroppedTwice(Box::new(123)), - Droppable::PanicOnDrop, - ]; - let _ = v.into_iter().take(1).collect::>(); + let v = vec![Old(0), Old(1), Old(2), Old(3), Old(4)]; + let _ = v.into_iter().map(|x| New(x.0)).take(2).collect::>(); })); - assert_eq!(unsafe { DROP_COUNTER_SHOULD_BE_DROPPED }, 1); - assert_eq!(unsafe { DROP_COUNTER_DROPPED_TWICE }, 1); + assert_eq!(unsafe { DROP_COUNTER_OLD[0] }, 1); + assert_eq!(unsafe { DROP_COUNTER_OLD[1] }, 1); + assert_eq!(unsafe { DROP_COUNTER_OLD[2] }, 1); + assert_eq!(unsafe { DROP_COUNTER_OLD[3] }, 1); + assert_eq!(unsafe { DROP_COUNTER_OLD[4] }, 1); + + assert_eq!(unsafe { DROP_COUNTER_NEW[0] }, 1); + assert_eq!(unsafe { DROP_COUNTER_NEW[1] }, 1); } // regression test for issue #85322. Peekable previously implemented InPlaceIterable, From c7d1ec009c56546268a84525d1cb0797d68997c1 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 2 Oct 2022 19:20:49 +0000 Subject: [PATCH 06/13] Don't ICE when trying to copy unsized value in const prop --- .../rustc_const_eval/src/interpret/place.rs | 16 +++++++++---- src/test/ui/const_prop/issue-102553.rs | 24 +++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/const_prop/issue-102553.rs diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 7a01b85381a3f..eeeb7d6d3e5cc 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/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 { diff --git a/src/test/ui/const_prop/issue-102553.rs b/src/test/ui/const_prop/issue-102553.rs new file mode 100644 index 0000000000000..523a9d7ac7204 --- /dev/null +++ b/src/test/ui/const_prop/issue-102553.rs @@ -0,0 +1,24 @@ +// compile-flags: --crate-type=lib +// check-pass + +pub trait Widget { + fn boxed<'w>(self) -> Box + 'w> + where + Self: Sized + 'w; +} + +pub trait WidgetDyn {} + +impl WidgetDyn for T where T: Widget {} + +impl Widget for dyn WidgetDyn + '_ { + fn boxed<'w>(self) -> Box + 'w> + where + Self: Sized + 'w, + { + // Even though this is illegal to const evaluate, this should never + // trigger an ICE because it can never be called from actual code + // (due to the trivially false where-clause predicate). + Box::new(self) + } +} From d0d6af91467ef60f12396f5d40a09eb4de8cb8b7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 2 Oct 2022 04:45:54 +0000 Subject: [PATCH 07/13] Lint for unsatisfied nested opaques --- .../locales/en-US/lint.ftl | 4 + compiler/rustc_lint/src/lib.rs | 3 + .../src/rpit_hidden_inferred_bound.rs | 143 ++++++++++++++++++ compiler/rustc_middle/src/lint.rs | 4 +- 4 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 compiler/rustc_lint/src/rpit_hidden_inferred_bound.rs diff --git a/compiler/rustc_error_messages/locales/en-US/lint.ftl b/compiler/rustc_error_messages/locales/en-US/lint.ftl index 80b0b1a8904a1..31c9a845e7e9d 100644 --- a/compiler/rustc_error_messages/locales/en-US/lint.ftl +++ b/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_rpit_hidden_inferred_bound = return-position `{$ty}` does not satisfy its associated type bounds + .specifically = this associated type bound is unsatisfied for `{$proj_ty}` + .suggestion = add this bound diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 4408f68dd63f3..fd7ea5a4bb5a2 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -65,6 +65,7 @@ mod noop_method_call; mod pass_by_value; mod passes; mod redundant_semicolon; +mod rpit_hidden_inferred_bound; mod traits; mod types; mod unused; @@ -95,6 +96,7 @@ use nonstandard_style::*; use noop_method_call::*; use pass_by_value::*; use redundant_semicolon::*; +use rpit_hidden_inferred_bound::*; use traits::*; use types::*; use unused::*; @@ -223,6 +225,7 @@ macro_rules! late_lint_mod_passes { EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums, InvalidAtomicOrdering: InvalidAtomicOrdering, NamedAsmLabels: NamedAsmLabels, + RpitHiddenInferredBound: RpitHiddenInferredBound, ] ); }; diff --git a/compiler/rustc_lint/src/rpit_hidden_inferred_bound.rs b/compiler/rustc_lint/src/rpit_hidden_inferred_bound.rs new file mode 100644 index 0000000000000..fd9872c80a945 --- /dev/null +++ b/compiler/rustc_lint/src/rpit_hidden_inferred_bound.rs @@ -0,0 +1,143 @@ +use hir::def_id::LocalDefId; +use rustc_hir as hir; +use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; +use rustc_macros::LintDiagnostic; +use rustc_middle::ty::{ + self, fold::BottomUpFolder, Ty, TypeFoldable, TypeSuperVisitable, TypeVisitable, TypeVisitor, +}; +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 `rpit_hidden_inferred_bound` lint detects cases in which nested RPITs + /// in associated type bounds are not written generally enough to satisfy the + /// bounds of the associated type. This functionality was removed in #97346, + /// but then rolled back in #99860 because it was made into a hard error too + /// quickly. + /// + /// 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. + pub RPIT_HIDDEN_INFERRED_BOUND, + Warn, + "detects the use of nested RPITs in associated type bounds that are not general enough" +} + +declare_lint_pass!(RpitHiddenInferredBound => [RPIT_HIDDEN_INFERRED_BOUND]); + +impl<'tcx> LateLintPass<'tcx> for RpitHiddenInferredBound { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: hir::intravisit::FnKind<'tcx>, + _: &'tcx hir::FnDecl<'tcx>, + _: &'tcx hir::Body<'tcx>, + _: rustc_span::Span, + id: hir::HirId, + ) { + if matches!(kind, hir::intravisit::FnKind::Closure) { + return; + } + + let fn_def_id = cx.tcx.hir().local_def_id(id); + let sig: ty::FnSig<'tcx> = + cx.tcx.liberate_late_bound_regions(fn_def_id.to_def_id(), cx.tcx.fn_sig(fn_def_id)); + cx.tcx.infer_ctxt().enter(|ref infcx| { + sig.output().visit_with(&mut VisitOpaqueBounds { infcx, cx, fn_def_id }); + }); + } +} + +struct VisitOpaqueBounds<'a, 'cx, 'tcx> { + infcx: &'a InferCtxt<'a, 'tcx>, + cx: &'cx LateContext<'tcx>, + fn_def_id: LocalDefId, +} + +impl<'tcx> TypeVisitor<'tcx> for VisitOpaqueBounds<'_, '_, 'tcx> { + fn visit_ty(&mut self, ty: Ty<'tcx>) -> std::ops::ControlFlow { + if let ty::Opaque(def_id, substs) = *ty.kind() + && let Some(hir::Node::Item(item)) = self.cx.tcx.hir().get_if_local(def_id) + && let hir::ItemKind::OpaqueTy(opaque) = &item.kind + && let hir::OpaqueTyOrigin::FnReturn(origin_def_id) = opaque.origin + && origin_def_id == self.fn_def_id + { + for pred_and_span in self.cx.tcx.bound_explicit_item_bounds(def_id).transpose_iter() { + let pred_span = pred_and_span.0.1; + let predicate = self.cx.tcx.liberate_late_bound_regions( + def_id, + pred_and_span.map_bound(|(pred, _)| *pred).subst(self.cx.tcx, substs).kind(), + ); + let ty::PredicateKind::Projection(proj) = predicate else { + continue; + }; + let Some(proj_term) = proj.term.ty() else { continue }; + + let proj_ty = self + .cx + .tcx + .mk_projection(proj.projection_ty.item_def_id, proj.projection_ty.substs); + let proj_replacer = &mut BottomUpFolder { + tcx: self.cx.tcx, + ty_op: |ty| if ty == proj_ty { proj_term } else { ty }, + lt_op: |lt| lt, + ct_op: |ct| ct, + }; + for assoc_pred_and_span in self + .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(self.cx.tcx, &proj.projection_ty.substs) + .fold_with(proj_replacer); + if !self.infcx.predicate_must_hold_modulo_regions(&traits::Obligation::new( + traits::ObligationCause::dummy(), + self.cx.param_env, + assoc_pred, + )) { + 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(self.cx.tcx.def_span(def_id).shrink_to_hi()), + ), + _ => (String::new(), None), + }; + self.cx.emit_spanned_lint( + RPIT_HIDDEN_INFERRED_BOUND, + pred_span, + RpitHiddenInferredBoundLint { + ty, + proj_ty: proj_term, + assoc_pred_span, + suggestion, + suggest_span, + }, + ); + } + } + } + } + + ty.super_visit_with(self) + } +} + +#[derive(LintDiagnostic)] +#[diag(lint::rpit_hidden_inferred_bound)] +struct RpitHiddenInferredBoundLint<'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, + suggestion: String, +} diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index f3e4f1faeb05c..1fb27fd87e87e 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -444,7 +444,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. From 426424b3200022eba361e171ba84be7c1e7cc837 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 2 Oct 2022 05:45:15 +0000 Subject: [PATCH 08/13] Make it a lint for all opaque types --- .../locales/en-US/lint.ftl | 2 +- compiler/rustc_lint/src/lib.rs | 6 +- .../src/opaque_hidden_inferred_bound.rs | 125 +++++++++++++++ .../src/rpit_hidden_inferred_bound.rs | 143 ------------------ .../ui/impl-trait/nested-return-type2-tait.rs | 1 + .../nested-return-type2-tait.stderr | 17 +++ src/test/ui/impl-trait/nested-return-type2.rs | 1 + .../ui/impl-trait/nested-return-type2.stderr | 17 +++ .../ui/impl-trait/nested-return-type3-tait.rs | 1 + .../nested-return-type3-tait.stderr | 17 +++ .../impl-trait/nested-return-type3-tait2.rs | 1 + .../nested-return-type3-tait2.stderr | 17 +++ .../impl-trait/nested-return-type3-tait3.rs | 1 + .../nested-return-type3-tait3.stderr | 17 +++ src/test/ui/impl-trait/nested-return-type3.rs | 1 + .../ui/impl-trait/nested-return-type3.stderr | 17 +++ 16 files changed, 237 insertions(+), 147 deletions(-) create mode 100644 compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs delete mode 100644 compiler/rustc_lint/src/rpit_hidden_inferred_bound.rs create mode 100644 src/test/ui/impl-trait/nested-return-type2-tait.stderr create mode 100644 src/test/ui/impl-trait/nested-return-type2.stderr create mode 100644 src/test/ui/impl-trait/nested-return-type3-tait.stderr create mode 100644 src/test/ui/impl-trait/nested-return-type3-tait2.stderr create mode 100644 src/test/ui/impl-trait/nested-return-type3-tait3.stderr create mode 100644 src/test/ui/impl-trait/nested-return-type3.stderr diff --git a/compiler/rustc_error_messages/locales/en-US/lint.ftl b/compiler/rustc_error_messages/locales/en-US/lint.ftl index 31c9a845e7e9d..0fd9b0ead167c 100644 --- a/compiler/rustc_error_messages/locales/en-US/lint.ftl +++ b/compiler/rustc_error_messages/locales/en-US/lint.ftl @@ -434,6 +434,6 @@ 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_rpit_hidden_inferred_bound = return-position `{$ty}` does not satisfy its associated type bounds +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 diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index fd7ea5a4bb5a2..9148c42195fbe 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -62,10 +62,10 @@ 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; -mod rpit_hidden_inferred_bound; mod traits; mod types; mod unused; @@ -94,9 +94,9 @@ 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 rpit_hidden_inferred_bound::*; use traits::*; use types::*; use unused::*; @@ -225,7 +225,7 @@ macro_rules! late_lint_mod_passes { EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums, InvalidAtomicOrdering: InvalidAtomicOrdering, NamedAsmLabels: NamedAsmLabels, - RpitHiddenInferredBound: RpitHiddenInferredBound, + OpaqueHiddenInferredBound: OpaqueHiddenInferredBound, ] ); }; diff --git a/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs b/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs new file mode 100644 index 0000000000000..7f18bf0018f6f --- /dev/null +++ b/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs @@ -0,0 +1,125 @@ +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. This functionality was + /// removed in #97346, but then rolled back in #99860 because it was made + /// into a hard error too quickly. + /// + /// 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. + 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`, for all of the bounds on `Assoc`, + // e.g. `type Assoc: OtherTrait`, replace `::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, + suggestion: String, +} diff --git a/compiler/rustc_lint/src/rpit_hidden_inferred_bound.rs b/compiler/rustc_lint/src/rpit_hidden_inferred_bound.rs deleted file mode 100644 index fd9872c80a945..0000000000000 --- a/compiler/rustc_lint/src/rpit_hidden_inferred_bound.rs +++ /dev/null @@ -1,143 +0,0 @@ -use hir::def_id::LocalDefId; -use rustc_hir as hir; -use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; -use rustc_macros::LintDiagnostic; -use rustc_middle::ty::{ - self, fold::BottomUpFolder, Ty, TypeFoldable, TypeSuperVisitable, TypeVisitable, TypeVisitor, -}; -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 `rpit_hidden_inferred_bound` lint detects cases in which nested RPITs - /// in associated type bounds are not written generally enough to satisfy the - /// bounds of the associated type. This functionality was removed in #97346, - /// but then rolled back in #99860 because it was made into a hard error too - /// quickly. - /// - /// 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. - pub RPIT_HIDDEN_INFERRED_BOUND, - Warn, - "detects the use of nested RPITs in associated type bounds that are not general enough" -} - -declare_lint_pass!(RpitHiddenInferredBound => [RPIT_HIDDEN_INFERRED_BOUND]); - -impl<'tcx> LateLintPass<'tcx> for RpitHiddenInferredBound { - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - kind: hir::intravisit::FnKind<'tcx>, - _: &'tcx hir::FnDecl<'tcx>, - _: &'tcx hir::Body<'tcx>, - _: rustc_span::Span, - id: hir::HirId, - ) { - if matches!(kind, hir::intravisit::FnKind::Closure) { - return; - } - - let fn_def_id = cx.tcx.hir().local_def_id(id); - let sig: ty::FnSig<'tcx> = - cx.tcx.liberate_late_bound_regions(fn_def_id.to_def_id(), cx.tcx.fn_sig(fn_def_id)); - cx.tcx.infer_ctxt().enter(|ref infcx| { - sig.output().visit_with(&mut VisitOpaqueBounds { infcx, cx, fn_def_id }); - }); - } -} - -struct VisitOpaqueBounds<'a, 'cx, 'tcx> { - infcx: &'a InferCtxt<'a, 'tcx>, - cx: &'cx LateContext<'tcx>, - fn_def_id: LocalDefId, -} - -impl<'tcx> TypeVisitor<'tcx> for VisitOpaqueBounds<'_, '_, 'tcx> { - fn visit_ty(&mut self, ty: Ty<'tcx>) -> std::ops::ControlFlow { - if let ty::Opaque(def_id, substs) = *ty.kind() - && let Some(hir::Node::Item(item)) = self.cx.tcx.hir().get_if_local(def_id) - && let hir::ItemKind::OpaqueTy(opaque) = &item.kind - && let hir::OpaqueTyOrigin::FnReturn(origin_def_id) = opaque.origin - && origin_def_id == self.fn_def_id - { - for pred_and_span in self.cx.tcx.bound_explicit_item_bounds(def_id).transpose_iter() { - let pred_span = pred_and_span.0.1; - let predicate = self.cx.tcx.liberate_late_bound_regions( - def_id, - pred_and_span.map_bound(|(pred, _)| *pred).subst(self.cx.tcx, substs).kind(), - ); - let ty::PredicateKind::Projection(proj) = predicate else { - continue; - }; - let Some(proj_term) = proj.term.ty() else { continue }; - - let proj_ty = self - .cx - .tcx - .mk_projection(proj.projection_ty.item_def_id, proj.projection_ty.substs); - let proj_replacer = &mut BottomUpFolder { - tcx: self.cx.tcx, - ty_op: |ty| if ty == proj_ty { proj_term } else { ty }, - lt_op: |lt| lt, - ct_op: |ct| ct, - }; - for assoc_pred_and_span in self - .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(self.cx.tcx, &proj.projection_ty.substs) - .fold_with(proj_replacer); - if !self.infcx.predicate_must_hold_modulo_regions(&traits::Obligation::new( - traits::ObligationCause::dummy(), - self.cx.param_env, - assoc_pred, - )) { - 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(self.cx.tcx.def_span(def_id).shrink_to_hi()), - ), - _ => (String::new(), None), - }; - self.cx.emit_spanned_lint( - RPIT_HIDDEN_INFERRED_BOUND, - pred_span, - RpitHiddenInferredBoundLint { - ty, - proj_ty: proj_term, - assoc_pred_span, - suggestion, - suggest_span, - }, - ); - } - } - } - } - - ty.super_visit_with(self) - } -} - -#[derive(LintDiagnostic)] -#[diag(lint::rpit_hidden_inferred_bound)] -struct RpitHiddenInferredBoundLint<'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, - suggestion: String, -} diff --git a/src/test/ui/impl-trait/nested-return-type2-tait.rs b/src/test/ui/impl-trait/nested-return-type2-tait.rs index 42613d5ccd9a4..089018a1cdf01 100644 --- a/src/test/ui/impl-trait/nested-return-type2-tait.rs +++ b/src/test/ui/impl-trait/nested-return-type2-tait.rs @@ -26,6 +26,7 @@ type Sendable = impl Send; // var to make it uphold the `: Duh` bound on `Trait::Assoc`. The opaque // type does not implement `Duh`, but if its hidden type does. fn foo() -> impl Trait { + //~^ WARN opaque type `impl Trait` does not satisfy its associated type bounds || 42 } diff --git a/src/test/ui/impl-trait/nested-return-type2-tait.stderr b/src/test/ui/impl-trait/nested-return-type2-tait.stderr new file mode 100644 index 0000000000000..a8eb69cfcb736 --- /dev/null +++ b/src/test/ui/impl-trait/nested-return-type2-tait.stderr @@ -0,0 +1,17 @@ +warning: opaque type `impl Trait` does not satisfy its associated type bounds + --> $DIR/nested-return-type2-tait.rs:28:24 + | +LL | type Assoc: Duh; + | --- this associated type bound is unsatisfied for `Sendable` +... +LL | fn foo() -> impl Trait { + | ^^^^^^^^^^^^^^^^ + | + = note: `#[warn(opaque_hidden_inferred_bound)]` on by default +help: add this bound + | +LL | type Sendable = impl Send + Duh; + | +++++ + +warning: 1 warning emitted + diff --git a/src/test/ui/impl-trait/nested-return-type2.rs b/src/test/ui/impl-trait/nested-return-type2.rs index 39928d543e15d..cc1f1f4ec44c8 100644 --- a/src/test/ui/impl-trait/nested-return-type2.rs +++ b/src/test/ui/impl-trait/nested-return-type2.rs @@ -23,6 +23,7 @@ impl R> Trait for F { // Lazy TAIT would error out, but we inserted a hack to make it work again, // keeping backwards compatibility. fn foo() -> impl Trait { + //~^ WARN opaque type `impl Trait` does not satisfy its associated type bounds || 42 } diff --git a/src/test/ui/impl-trait/nested-return-type2.stderr b/src/test/ui/impl-trait/nested-return-type2.stderr new file mode 100644 index 0000000000000..3aed05ca13298 --- /dev/null +++ b/src/test/ui/impl-trait/nested-return-type2.stderr @@ -0,0 +1,17 @@ +warning: opaque type `impl Trait` does not satisfy its associated type bounds + --> $DIR/nested-return-type2.rs:25:24 + | +LL | type Assoc: Duh; + | --- this associated type bound is unsatisfied for `impl Send` +... +LL | fn foo() -> impl Trait { + | ^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(opaque_hidden_inferred_bound)]` on by default +help: add this bound + | +LL | fn foo() -> impl Trait { + | +++++ + +warning: 1 warning emitted + diff --git a/src/test/ui/impl-trait/nested-return-type3-tait.rs b/src/test/ui/impl-trait/nested-return-type3-tait.rs index 3936f4dbbb426..3a97e35b4c400 100644 --- a/src/test/ui/impl-trait/nested-return-type3-tait.rs +++ b/src/test/ui/impl-trait/nested-return-type3-tait.rs @@ -17,6 +17,7 @@ impl Trait for F { type Sendable = impl Send; fn foo() -> impl Trait { + //~^ WARN opaque type `impl Trait` does not satisfy its associated type bounds 42 } diff --git a/src/test/ui/impl-trait/nested-return-type3-tait.stderr b/src/test/ui/impl-trait/nested-return-type3-tait.stderr new file mode 100644 index 0000000000000..5f58c8dca4ad4 --- /dev/null +++ b/src/test/ui/impl-trait/nested-return-type3-tait.stderr @@ -0,0 +1,17 @@ +warning: opaque type `impl Trait` does not satisfy its associated type bounds + --> $DIR/nested-return-type3-tait.rs:19:24 + | +LL | type Assoc: Duh; + | --- this associated type bound is unsatisfied for `Sendable` +... +LL | fn foo() -> impl Trait { + | ^^^^^^^^^^^^^^^^ + | + = note: `#[warn(opaque_hidden_inferred_bound)]` on by default +help: add this bound + | +LL | type Sendable = impl Send + Duh; + | +++++ + +warning: 1 warning emitted + diff --git a/src/test/ui/impl-trait/nested-return-type3-tait2.rs b/src/test/ui/impl-trait/nested-return-type3-tait2.rs index 56778ed90dc7b..5b6f78a989687 100644 --- a/src/test/ui/impl-trait/nested-return-type3-tait2.rs +++ b/src/test/ui/impl-trait/nested-return-type3-tait2.rs @@ -16,6 +16,7 @@ impl Trait for F { type Sendable = impl Send; type Traitable = impl Trait; +//~^ WARN opaque type `Traitable` does not satisfy its associated type bounds fn foo() -> Traitable { 42 diff --git a/src/test/ui/impl-trait/nested-return-type3-tait2.stderr b/src/test/ui/impl-trait/nested-return-type3-tait2.stderr new file mode 100644 index 0000000000000..c07f6ead75028 --- /dev/null +++ b/src/test/ui/impl-trait/nested-return-type3-tait2.stderr @@ -0,0 +1,17 @@ +warning: opaque type `Traitable` does not satisfy its associated type bounds + --> $DIR/nested-return-type3-tait2.rs:18:29 + | +LL | type Assoc: Duh; + | --- this associated type bound is unsatisfied for `Sendable` +... +LL | type Traitable = impl Trait; + | ^^^^^^^^^^^^^^^^ + | + = note: `#[warn(opaque_hidden_inferred_bound)]` on by default +help: add this bound + | +LL | type Sendable = impl Send + Duh; + | +++++ + +warning: 1 warning emitted + diff --git a/src/test/ui/impl-trait/nested-return-type3-tait3.rs b/src/test/ui/impl-trait/nested-return-type3-tait3.rs index 04c6c92b1a3c5..394d8f581102f 100644 --- a/src/test/ui/impl-trait/nested-return-type3-tait3.rs +++ b/src/test/ui/impl-trait/nested-return-type3-tait3.rs @@ -15,6 +15,7 @@ impl Trait for F { } type Traitable = impl Trait; +//~^ WARN opaque type `Traitable` does not satisfy its associated type bounds fn foo() -> Traitable { 42 diff --git a/src/test/ui/impl-trait/nested-return-type3-tait3.stderr b/src/test/ui/impl-trait/nested-return-type3-tait3.stderr new file mode 100644 index 0000000000000..d98ad89222fa7 --- /dev/null +++ b/src/test/ui/impl-trait/nested-return-type3-tait3.stderr @@ -0,0 +1,17 @@ +warning: opaque type `Traitable` does not satisfy its associated type bounds + --> $DIR/nested-return-type3-tait3.rs:17:29 + | +LL | type Assoc: Duh; + | --- this associated type bound is unsatisfied for `impl Send` +... +LL | type Traitable = impl Trait; + | ^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(opaque_hidden_inferred_bound)]` on by default +help: add this bound + | +LL | type Traitable = impl Trait; + | +++++ + +warning: 1 warning emitted + diff --git a/src/test/ui/impl-trait/nested-return-type3.rs b/src/test/ui/impl-trait/nested-return-type3.rs index 74b4dae22ebfd..5a764fc4c285a 100644 --- a/src/test/ui/impl-trait/nested-return-type3.rs +++ b/src/test/ui/impl-trait/nested-return-type3.rs @@ -13,6 +13,7 @@ impl Trait for F { } fn foo() -> impl Trait { + //~^ WARN opaque type `impl Trait` does not satisfy its associated type bounds 42 } diff --git a/src/test/ui/impl-trait/nested-return-type3.stderr b/src/test/ui/impl-trait/nested-return-type3.stderr new file mode 100644 index 0000000000000..632de71aa4c88 --- /dev/null +++ b/src/test/ui/impl-trait/nested-return-type3.stderr @@ -0,0 +1,17 @@ +warning: opaque type `impl Trait` does not satisfy its associated type bounds + --> $DIR/nested-return-type3.rs:15:24 + | +LL | type Assoc: Duh; + | --- this associated type bound is unsatisfied for `impl Send` +... +LL | fn foo() -> impl Trait { + | ^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(opaque_hidden_inferred_bound)]` on by default +help: add this bound + | +LL | fn foo() -> impl Trait { + | +++++ + +warning: 1 warning emitted + From 7a8854037b81e04bea5309e5b107bfe09fc841c0 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 2 Oct 2022 19:07:31 +0000 Subject: [PATCH 09/13] Add example to opaque_hidden_inferred_bound lint --- .../src/opaque_hidden_inferred_bound.rs | 43 ++++++++++++++++--- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs b/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs index 7f18bf0018f6f..d8ce20db37ce9 100644 --- a/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs +++ b/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs @@ -11,13 +11,44 @@ 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. This functionality was - /// removed in #97346, but then rolled back in #99860 because it was made - /// into a hard error too quickly. + /// to satisfy the bounds of the associated type. /// - /// 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. + /// ### 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 { + /// 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" From 1750c7bdd36ec18324423bd30867e39d787d5977 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sat, 10 Sep 2022 15:00:37 +0200 Subject: [PATCH 10/13] Clarify documentation --- library/alloc/src/vec/in_place_collect.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library/alloc/src/vec/in_place_collect.rs b/library/alloc/src/vec/in_place_collect.rs index 83ed3d915a291..9f2555ee1611a 100644 --- a/library/alloc/src/vec/in_place_collect.rs +++ b/library/alloc/src/vec/in_place_collect.rs @@ -194,11 +194,10 @@ 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 try to drop the destination buffer and its elements. + // 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 - // and won't panic before that. + // 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 From 1456f73bb4a276c49b9caa6087cf4305536d1f74 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Mon, 3 Oct 2022 21:01:19 +0200 Subject: [PATCH 11/13] Fix rustdoc ICE in invalid_rust_codeblocks lint The diagnostic message extraction code didn't handle translations yet. --- .../passes/check_code_block_syntax.rs | 7 +++++-- src/test/rustdoc-ui/invalid-syntax.rs | 6 ++++++ src/test/rustdoc-ui/invalid-syntax.stderr | 17 ++++++++++++++++- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/passes/check_code_block_syntax.rs b/src/librustdoc/passes/check_code_block_syntax.rs index 23f87838544df..14a38a760d238 100644 --- a/src/librustdoc/passes/check_code_block_syntax.rs +++ b/src/librustdoc/passes/check_code_block_syntax.rs @@ -192,8 +192,11 @@ impl Translate for BufferEmitter { impl Emitter for BufferEmitter { fn emit_diagnostic(&mut self, diag: &Diagnostic) { let mut buffer = self.buffer.borrow_mut(); - // FIXME(davidtwco): need to support translation here eventually - buffer.messages.push(format!("error from rustc: {}", diag.message[0].0.expect_str())); + + let fluent_args = self.to_fluent_args(diag.args()); + let translated_main_message = self.translate_message(&diag.message[0].0, &fluent_args); + + buffer.messages.push(format!("error from rustc: {}", translated_main_message)); if diag.is_error() { buffer.has_errors = true; } diff --git a/src/test/rustdoc-ui/invalid-syntax.rs b/src/test/rustdoc-ui/invalid-syntax.rs index b503d1093fda8..acb2a6f084f23 100644 --- a/src/test/rustdoc-ui/invalid-syntax.rs +++ b/src/test/rustdoc-ui/invalid-syntax.rs @@ -99,3 +99,9 @@ pub fn indent_after_fenced() {} /// ``` pub fn invalid() {} //~^^^^ WARNING could not parse code block as Rust code + +/// ``` +/// fn wook_at_my_beautifuw_bwaces_plz() {); +/// ``` +pub fn uwu() {} +//~^^^^ WARNING could not parse code block as Rust code diff --git a/src/test/rustdoc-ui/invalid-syntax.stderr b/src/test/rustdoc-ui/invalid-syntax.stderr index 6388830cf1bae..597d19e748cb7 100644 --- a/src/test/rustdoc-ui/invalid-syntax.stderr +++ b/src/test/rustdoc-ui/invalid-syntax.stderr @@ -150,5 +150,20 @@ help: mark blocks that do not contain Rust code as text LL | /// ```text | ++++ -warning: 12 warnings emitted +warning: could not parse code block as Rust code + --> $DIR/invalid-syntax.rs:103:5 + | +LL | /// ``` + | _____^ +LL | | /// fn wook_at_my_beautifuw_bwaces_plz() {); +LL | | /// ``` + | |_______^ + | + = note: error from rustc: mismatched closing delimiter: `)` +help: mark blocks that do not contain Rust code as text + | +LL | /// ```text + | ++++ + +warning: 13 warnings emitted From 8c600120e6abd8c25a88443451d190c68142180f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 29 Sep 2022 22:03:03 +0000 Subject: [PATCH 12/13] Normalize substs before resolving instance in NoopMethodCall lint --- compiler/rustc_lint/src/noop_method_call.rs | 11 +++++---- .../generic_const_exprs/issue-102074.rs | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/const-generics/generic_const_exprs/issue-102074.rs diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 19188d5c37651..2a3ff3a7546a4 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -46,7 +46,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 +56,22 @@ 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); + let substs = cx + .tcx + .normalize_erasing_regions(cx.param_env, 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); // 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. diff --git a/src/test/ui/const-generics/generic_const_exprs/issue-102074.rs b/src/test/ui/const-generics/generic_const_exprs/issue-102074.rs new file mode 100644 index 0000000000000..66d15cf1215da --- /dev/null +++ b/src/test/ui/const-generics/generic_const_exprs/issue-102074.rs @@ -0,0 +1,23 @@ +// check-pass +// Checks that the NoopMethodCall lint doesn't call Instance::resolve on unresolved consts + +#![feature(generic_const_exprs)] +#![allow(incomplete_features)] + +#[derive(Debug, Clone)] +pub struct Aes128CipherKey([u8; Aes128Cipher::KEY_LEN]); + +impl Aes128CipherKey { + pub fn new(key: &[u8; Aes128Cipher::KEY_LEN]) -> Self { + Self(key.clone()) + } +} + +#[derive(Debug, Clone)] +pub struct Aes128Cipher; + +impl Aes128Cipher { + const KEY_LEN: usize = 16; +} + +fn main() {} From e1b313af46b74a446d7772a261e006c199a5b2e0 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 4 Oct 2022 03:22:45 +0000 Subject: [PATCH 13/13] We are able to resolve methods even if they need subst --- compiler/rustc_lint/src/noop_method_call.rs | 6 ------ src/test/ui/lint/noop-method-call.rs | 1 + src/test/ui/lint/noop-method-call.stderr | 12 ++++++++++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 2a3ff3a7546a4..9a62afd3cafb7 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/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; @@ -65,11 +64,6 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { let substs = cx .tcx .normalize_erasing_regions(cx.param_env, 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; - } // Resolve the trait method instance. let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, cx.param_env, did, substs) else { return diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs index 9870c813572e3..89b2966359542 100644 --- a/src/test/ui/lint/noop-method-call.rs +++ b/src/test/ui/lint/noop-method-call.rs @@ -46,6 +46,7 @@ fn main() { fn generic(non_clone_type: &PlainType) { non_clone_type.clone(); + //~^ WARNING call to `.clone()` on a reference in this situation does nothing } fn non_generic(non_clone_type: &PlainType) { diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr index c71de44dc7168..6a904d01abc8e 100644 --- a/src/test/ui/lint/noop-method-call.stderr +++ b/src/test/ui/lint/noop-method-call.stderr @@ -28,12 +28,20 @@ LL | let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); = note: the type `&PlainType` which `borrow` is being called on is the same as the type returned from `borrow`, so the method call does not do anything and can be removed warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:52:19 + --> $DIR/noop-method-call.rs:48:19 + | +LL | non_clone_type.clone(); + | ^^^^^^^^ unnecessary method call + | + = note: the type `&PlainType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed + +warning: call to `.clone()` on a reference in this situation does nothing + --> $DIR/noop-method-call.rs:53:19 | LL | non_clone_type.clone(); | ^^^^^^^^ unnecessary method call | = note: the type `&PlainType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed -warning: 4 warnings emitted +warning: 5 warnings emitted