From 15fbcc363680ce100a563aac304c389d0f57dfd4 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 18 Nov 2023 19:29:39 +0000 Subject: [PATCH] Rework supertrait lint once again --- compiler/rustc_lint/messages.ftl | 3 +- .../src/deref_into_dyn_supertrait.rs | 42 +++++++++++-------- compiler/rustc_lint/src/lints.rs | 14 +++++-- .../trait-upcasting/deref-lint-regions.stderr | 2 +- tests/ui/traits/trait-upcasting/deref-lint.rs | 1 + .../traits/trait-upcasting/deref-lint.stderr | 4 +- .../deref-upcast-behavioral-change.rs | 34 +++++++++++++++ .../deref-upcast-behavioral-change.stderr | 16 +++++++ .../migrate-lint-different-substs.rs | 20 +++++++++ .../migrate-lint-different-substs.stderr | 14 +++++++ 10 files changed, 125 insertions(+), 25 deletions(-) create mode 100644 tests/ui/traits/trait-upcasting/deref-upcast-behavioral-change.rs create mode 100644 tests/ui/traits/trait-upcasting/deref-upcast-behavioral-change.stderr create mode 100644 tests/ui/traits/trait-upcasting/migrate-lint-different-substs.rs create mode 100644 tests/ui/traits/trait-upcasting/migrate-lint-different-substs.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 80b21bd8ece..2db610d640c 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -491,8 +491,9 @@ lint_requested_level = requested on the command line with `{$level} {$lint_name} lint_span_use_eq_ctxt = use `.eq_ctxt()` instead of `.ctxt() == .ctxt()` lint_supertrait_as_deref_target = this `Deref` implementation is covered by an implicit supertrait coercion + .label = `{$self_ty}` implements `Deref` which conflicts with supertrait `{$supertrait_principal}` + .label2 = target type is a supertrait of `{$self_ty}` .help = consider removing this implementation or replacing it with a method instead - .label = target type is a supertrait of `{$t}` lint_suspicious_double_ref_clone = using `.clone()` on a double reference, which returns `{$ty}` instead of cloning the inner type diff --git a/compiler/rustc_lint/src/deref_into_dyn_supertrait.rs b/compiler/rustc_lint/src/deref_into_dyn_supertrait.rs index 93c7c37c655..98bafc0f263 100644 --- a/compiler/rustc_lint/src/deref_into_dyn_supertrait.rs +++ b/compiler/rustc_lint/src/deref_into_dyn_supertrait.rs @@ -53,35 +53,43 @@ impl<'tcx> LateLintPass<'tcx> for DerefIntoDynSupertrait { let tcx = cx.tcx; // `Deref` is being implemented for `t` if let hir::ItemKind::Impl(impl_) = item.kind + // the trait is a `Deref` implementation && let Some(trait_) = &impl_.of_trait - && let t = tcx.type_of(item.owner_id).instantiate_identity() - && let opt_did @ Some(did) = trait_.trait_def_id() - && opt_did == tcx.lang_items().deref_trait() - // `t` is `dyn t_principal` - && let ty::Dynamic(data, _, ty::Dyn) = t.kind() - && let Some(t_principal) = data.principal() + && let Some(did) = trait_.trait_def_id() + && Some(did) == tcx.lang_items().deref_trait() + // the self type is `dyn t_principal` + && let self_ty = tcx.type_of(item.owner_id).instantiate_identity() + && let ty::Dynamic(data, _, ty::Dyn) = self_ty.kind() + && let Some(self_principal) = data.principal() // `::Target` is `dyn target_principal` - && let Some(target) = cx.get_associated_type(t, did, "Target") + && let Some(target) = cx.get_associated_type(self_ty, did, "Target") && let ty::Dynamic(data, _, ty::Dyn) = target.kind() && let Some(target_principal) = data.principal() // `target_principal` is a supertrait of `t_principal` - && supertraits(tcx, t_principal.with_self_ty(tcx, tcx.types.trait_object_dummy_self)) - .any(|sup| { - tcx.erase_regions( - sup.map_bound(|x| ty::ExistentialTraitRef::erase_self_ty(tcx, x)), - ) == tcx.erase_regions(target_principal) - }) + && let Some(supertrait_principal) = supertraits(tcx, self_principal.with_self_ty(tcx, self_ty)) + .find(|supertrait| supertrait.def_id() == target_principal.def_id()) { - let t = tcx.erase_regions(t); - let label = impl_ + // erase regions in self type for better diagnostic presentation + let (self_ty, target_principal, supertrait_principal) = + tcx.erase_regions((self_ty, target_principal, supertrait_principal)); + let label2 = impl_ .items .iter() .find_map(|i| (i.ident.name == sym::Target).then_some(i.span)) .map(|label| SupertraitAsDerefTargetLabel { label }); + let span = tcx.def_span(item.owner_id.def_id); cx.emit_spanned_lint( DEREF_INTO_DYN_SUPERTRAIT, - tcx.def_span(item.owner_id.def_id), - SupertraitAsDerefTarget { t, label }, + span, + SupertraitAsDerefTarget { + self_ty, + supertrait_principal: supertrait_principal.map_bound(|trait_ref| { + ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref) + }), + target_principal, + label: span, + label2, + }, ); } } diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 7fe488e1243..9fda53c2533 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -10,7 +10,9 @@ use rustc_errors::{ }; use rustc_hir::def_id::DefId; use rustc_macros::{LintDiagnostic, Subdiagnostic}; -use rustc_middle::ty::{inhabitedness::InhabitedPredicate, Clause, Ty, TyCtxt}; +use rustc_middle::ty::{ + inhabitedness::InhabitedPredicate, Clause, PolyExistentialTraitRef, Ty, TyCtxt, +}; use rustc_session::parse::ParseSess; use rustc_span::{edition::Edition, sym, symbol::Ident, Span, Symbol}; @@ -556,13 +558,17 @@ pub enum BuiltinSpecialModuleNameUsed { #[diag(lint_supertrait_as_deref_target)] #[help] pub struct SupertraitAsDerefTarget<'a> { - pub t: Ty<'a>, + pub self_ty: Ty<'a>, + pub supertrait_principal: PolyExistentialTraitRef<'a>, + pub target_principal: PolyExistentialTraitRef<'a>, + #[label] + pub label: Span, #[subdiagnostic] - pub label: Option, + pub label2: Option, } #[derive(Subdiagnostic)] -#[label(lint_label)] +#[label(lint_label2)] pub struct SupertraitAsDerefTargetLabel { #[primary_span] pub label: Span, diff --git a/tests/ui/traits/trait-upcasting/deref-lint-regions.stderr b/tests/ui/traits/trait-upcasting/deref-lint-regions.stderr index dd4aa8e9a07..557a4420a3d 100644 --- a/tests/ui/traits/trait-upcasting/deref-lint-regions.stderr +++ b/tests/ui/traits/trait-upcasting/deref-lint-regions.stderr @@ -2,7 +2,7 @@ warning: this `Deref` implementation is covered by an implicit supertrait coerci --> $DIR/deref-lint-regions.rs:8:1 | LL | impl<'a> Deref for dyn Foo<'a> { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `dyn Foo<'_>` implements `Deref>` which conflicts with supertrait `Bar<'_>` LL | LL | type Target = dyn Bar<'a>; | -------------------------- target type is a supertrait of `dyn Foo<'_>` diff --git a/tests/ui/traits/trait-upcasting/deref-lint.rs b/tests/ui/traits/trait-upcasting/deref-lint.rs index dfca7b0fa68..68838d2ae20 100644 --- a/tests/ui/traits/trait-upcasting/deref-lint.rs +++ b/tests/ui/traits/trait-upcasting/deref-lint.rs @@ -8,6 +8,7 @@ trait B: A {} impl<'a> Deref for dyn 'a + B { //~^ WARN this `Deref` implementation is covered by an implicit supertrait coercion + type Target = dyn A; fn deref(&self) -> &Self::Target { todo!() diff --git a/tests/ui/traits/trait-upcasting/deref-lint.stderr b/tests/ui/traits/trait-upcasting/deref-lint.stderr index 0f7a61dfa80..5a13659edf5 100644 --- a/tests/ui/traits/trait-upcasting/deref-lint.stderr +++ b/tests/ui/traits/trait-upcasting/deref-lint.stderr @@ -2,8 +2,8 @@ warning: this `Deref` implementation is covered by an implicit supertrait coerci --> $DIR/deref-lint.rs:9:1 | LL | impl<'a> Deref for dyn 'a + B { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -LL | + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `dyn B` implements `Deref` which conflicts with supertrait `A` +... LL | type Target = dyn A; | -------------------- target type is a supertrait of `dyn B` | diff --git a/tests/ui/traits/trait-upcasting/deref-upcast-behavioral-change.rs b/tests/ui/traits/trait-upcasting/deref-upcast-behavioral-change.rs new file mode 100644 index 00000000000..366eae1a58a --- /dev/null +++ b/tests/ui/traits/trait-upcasting/deref-upcast-behavioral-change.rs @@ -0,0 +1,34 @@ +#![deny(deref_into_dyn_supertrait)] +use std::ops::Deref; + +trait Bar {} +impl Bar for T {} + +trait Foo: Bar { + fn as_dyn_bar_u32<'a>(&self) -> &(dyn Bar + 'a); +} + +impl Foo for () { + fn as_dyn_bar_u32<'a>(&self) -> &(dyn Bar + 'a) { + self + } +} + +impl<'a> Deref for dyn Foo + 'a { + type Target = dyn Bar + 'a; + + fn deref(&self) -> &Self::Target { + self.as_dyn_bar_u32() + } +} + +fn take_dyn(x: &dyn Bar) -> T { + todo!() +} + +fn main() { + let x: &dyn Foo = &(); + let y = take_dyn(x); + let z: u32 = y; + //~^ ERROR mismatched types +} diff --git a/tests/ui/traits/trait-upcasting/deref-upcast-behavioral-change.stderr b/tests/ui/traits/trait-upcasting/deref-upcast-behavioral-change.stderr new file mode 100644 index 00000000000..e8bab322a38 --- /dev/null +++ b/tests/ui/traits/trait-upcasting/deref-upcast-behavioral-change.stderr @@ -0,0 +1,16 @@ +error[E0308]: mismatched types + --> $DIR/deref-upcast-behavioral-change.rs:32:18 + | +LL | let z: u32 = y; + | --- ^ expected `u32`, found `i32` + | | + | expected due to this + | +help: you can convert an `i32` to a `u32` and panic if the converted value doesn't fit + | +LL | let z: u32 = y.try_into().unwrap(); + | ++++++++++++++++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/traits/trait-upcasting/migrate-lint-different-substs.rs b/tests/ui/traits/trait-upcasting/migrate-lint-different-substs.rs new file mode 100644 index 00000000000..2c9126c863d --- /dev/null +++ b/tests/ui/traits/trait-upcasting/migrate-lint-different-substs.rs @@ -0,0 +1,20 @@ +// check-pass + +use std::ops::Deref; + +trait Bar {} + +trait Foo: Bar { + fn as_dyn_bar_u32<'a>(&self) -> &(dyn Bar + 'a); +} + +impl<'a> Deref for dyn Foo + 'a { + //~^ WARN this `Deref` implementation is covered by an implicit supertrait coercion + type Target = dyn Bar + 'a; + + fn deref(&self) -> &Self::Target { + self.as_dyn_bar_u32() + } +} + +fn main() {} diff --git a/tests/ui/traits/trait-upcasting/migrate-lint-different-substs.stderr b/tests/ui/traits/trait-upcasting/migrate-lint-different-substs.stderr new file mode 100644 index 00000000000..a447f9cf83b --- /dev/null +++ b/tests/ui/traits/trait-upcasting/migrate-lint-different-substs.stderr @@ -0,0 +1,14 @@ +warning: this `Deref` implementation is covered by an implicit supertrait coercion + --> $DIR/migrate-lint-different-substs.rs:11:1 + | +LL | impl<'a> Deref for dyn Foo + 'a { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `dyn Foo` implements `Deref>` which conflicts with supertrait `Bar` +LL | +LL | type Target = dyn Bar + 'a; + | -------------------------------- target type is a supertrait of `dyn Foo` + | + = help: consider removing this implementation or replacing it with a method instead + = note: `#[warn(deref_into_dyn_supertrait)]` on by default + +warning: 1 warning emitted +