From 816d811fd551cbd7f6ad5f45ec3ac3d017aa4de1 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 | 36 +++++++++++-------- compiler/rustc_lint/src/lints.rs | 4 ++- .../migrate-lint-deny-regions.rs | 2 +- .../migrate-lint-deny-regions.stderr | 3 +- .../trait-upcasting/migrate-lint-deny.rs | 2 +- .../trait-upcasting/migrate-lint-deny.stderr | 3 +- .../migrate-lint-different-substs.rs | 21 +++++++++++ .../migrate-lint-different-substs.stderr | 20 +++++++++++ 9 files changed, 73 insertions(+), 21 deletions(-) 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 068f1372c0ecd..aa437865118ce 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -496,8 +496,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 = `{$t}` implements `Deref` with supertrait `{$target_principal}` as target +lint_supertrait_as_deref_target = `{$self_ty}` implements `Deref` which conflicts with supertrait `{$supertrait_principal}` .label = target type is set here + .note = inference and runtime behavior may change after `#[feature(trait_upcasting)]` is enabled by default 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 41a70bfcc5936..cbb8e9136b74a 100644 --- a/compiler/rustc_lint/src/deref_into_dyn_supertrait.rs +++ b/compiler/rustc_lint/src/deref_into_dyn_supertrait.rs @@ -62,26 +62,25 @@ 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); + // 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 label = impl_ .items .iter() @@ -90,7 +89,14 @@ impl<'tcx> LateLintPass<'tcx> for DerefIntoDynSupertrait { cx.emit_spanned_lint( DEREF_INTO_DYN_SUPERTRAIT, tcx.def_span(item.owner_id.def_id), - SupertraitAsDerefTarget { t, target_principal, label }, + SupertraitAsDerefTarget { + self_ty, + supertrait_principal: supertrait_principal.map_bound(|trait_ref| { + ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref) + }), + target_principal, + label, + }, ); } } diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 756899e50a8cd..27b9de01db27d 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -571,8 +571,10 @@ pub struct BuiltinUnexpectedCliConfigValue { // deref_into_dyn_supertrait.rs #[derive(LintDiagnostic)] #[diag(lint_supertrait_as_deref_target)] +#[note] pub struct SupertraitAsDerefTarget<'a> { - pub t: Ty<'a>, + pub self_ty: Ty<'a>, + pub supertrait_principal: PolyExistentialTraitRef<'a>, pub target_principal: PolyExistentialTraitRef<'a>, #[subdiagnostic] pub label: Option, diff --git a/tests/ui/traits/trait-upcasting/migrate-lint-deny-regions.rs b/tests/ui/traits/trait-upcasting/migrate-lint-deny-regions.rs index e743eb0ecd385..8f2d0a38fcfd5 100644 --- a/tests/ui/traits/trait-upcasting/migrate-lint-deny-regions.rs +++ b/tests/ui/traits/trait-upcasting/migrate-lint-deny-regions.rs @@ -6,7 +6,7 @@ trait Bar<'a> {} trait Foo<'a>: Bar<'a> {} impl<'a> Deref for dyn Foo<'a> { - //~^ ERROR dyn Foo<'_>` implements `Deref` with supertrait `Bar<'_>` as target + //~^ ERROR `dyn Foo<'_>` implements `Deref>` which conflicts with supertrait `Bar<'_>` //~| WARN this was previously accepted by the compiler type Target = dyn Bar<'a>; diff --git a/tests/ui/traits/trait-upcasting/migrate-lint-deny-regions.stderr b/tests/ui/traits/trait-upcasting/migrate-lint-deny-regions.stderr index d6480bcc66bb6..8491b2fdc0c87 100644 --- a/tests/ui/traits/trait-upcasting/migrate-lint-deny-regions.stderr +++ b/tests/ui/traits/trait-upcasting/migrate-lint-deny-regions.stderr @@ -1,4 +1,4 @@ -error: `dyn Foo<'_>` implements `Deref` with supertrait `Bar<'_>` as target +error: `dyn Foo<'_>` implements `Deref>` which conflicts with supertrait `Bar<'_>` --> $DIR/migrate-lint-deny-regions.rs:8:1 | LL | impl<'a> Deref for dyn Foo<'a> { @@ -9,6 +9,7 @@ LL | type Target = dyn Bar<'a>; | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #89460 + = note: inference and runtime behavior may change after `#[feature(trait_upcasting)]` is enabled by default note: the lint level is defined here --> $DIR/migrate-lint-deny-regions.rs:1:9 | diff --git a/tests/ui/traits/trait-upcasting/migrate-lint-deny.rs b/tests/ui/traits/trait-upcasting/migrate-lint-deny.rs index 828aaec79ab08..ac81b887b4065 100644 --- a/tests/ui/traits/trait-upcasting/migrate-lint-deny.rs +++ b/tests/ui/traits/trait-upcasting/migrate-lint-deny.rs @@ -7,7 +7,7 @@ trait A {} trait B: A {} impl<'a> Deref for dyn 'a + B { - //~^ ERROR `dyn B` implements `Deref` with supertrait `A` as target + //~^ ERROR `dyn B` implements `Deref` which conflicts with supertrait `A` //~| WARN this was previously accepted by the compiler but is being phased out; type Target = dyn A; diff --git a/tests/ui/traits/trait-upcasting/migrate-lint-deny.stderr b/tests/ui/traits/trait-upcasting/migrate-lint-deny.stderr index 522b35299e9a0..be3bd21eead2b 100644 --- a/tests/ui/traits/trait-upcasting/migrate-lint-deny.stderr +++ b/tests/ui/traits/trait-upcasting/migrate-lint-deny.stderr @@ -1,4 +1,4 @@ -error: `dyn B` implements `Deref` with supertrait `A` as target +error: `dyn B` implements `Deref` which conflicts with supertrait `A` --> $DIR/migrate-lint-deny.rs:9:1 | LL | impl<'a> Deref for dyn 'a + B { @@ -9,6 +9,7 @@ LL | type Target = dyn A; | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #89460 + = note: inference and runtime behavior may change after `#[feature(trait_upcasting)]` is enabled by default note: the lint level is defined here --> $DIR/migrate-lint-deny.rs:1:9 | 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 0000000000000..a4afbc76ee81c --- /dev/null +++ b/tests/ui/traits/trait-upcasting/migrate-lint-different-substs.rs @@ -0,0 +1,21 @@ +#![deny(deref_into_dyn_supertrait)] + +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 { + //~^ ERROR `dyn Foo` implements `Deref>` which conflicts with supertrait `Bar` + //~| WARN this was previously accepted by the compiler + 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 0000000000000..245391f3fdfeb --- /dev/null +++ b/tests/ui/traits/trait-upcasting/migrate-lint-different-substs.stderr @@ -0,0 +1,20 @@ +error: `dyn Foo` implements `Deref>` which conflicts with supertrait `Bar` + --> $DIR/migrate-lint-different-substs.rs:11:1 + | +LL | impl<'a> Deref for dyn Foo + 'a { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | type Target = dyn Bar + 'a; + | -------------------------------- target type is set here + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #89460 + = note: inference and runtime behavior may change after `#[feature(trait_upcasting)]` is enabled by default +note: the lint level is defined here + --> $DIR/migrate-lint-different-substs.rs:1:9 + | +LL | #![deny(deref_into_dyn_supertrait)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error +