Skip to content

Rework supertrait lint once again #118146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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<Target = dyn {$target_principal}>` 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
Expand Down
42 changes: 25 additions & 17 deletions compiler/rustc_lint/src/deref_into_dyn_supertrait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
// `<T as Deref>::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,
},
);
}
}
Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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<SupertraitAsDerefTargetLabel>,
pub label2: Option<SupertraitAsDerefTargetLabel>,
}

#[derive(Subdiagnostic)]
#[label(lint_label)]
#[label(lint_label2)]
pub struct SupertraitAsDerefTargetLabel {
#[primary_span]
pub label: Span,
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/traits/trait-upcasting/deref-lint-regions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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<Target = dyn Bar<'_>>` which conflicts with supertrait `Bar<'_>`
LL |
LL | type Target = dyn Bar<'a>;
| -------------------------- target type is a supertrait of `dyn Foo<'_>`
Expand Down
1 change: 1 addition & 0 deletions tests/ui/traits/trait-upcasting/deref-lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!()
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/traits/trait-upcasting/deref-lint.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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<Target = dyn A>` which conflicts with supertrait `A`
...
LL | type Target = dyn A;
| -------------------- target type is a supertrait of `dyn B`
|
Expand Down
34 changes: 34 additions & 0 deletions tests/ui/traits/trait-upcasting/deref-upcast-behavioral-change.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#![deny(deref_into_dyn_supertrait)]
use std::ops::Deref;

trait Bar<T> {}
impl<T, U> Bar<U> for T {}

trait Foo: Bar<i32> {
fn as_dyn_bar_u32<'a>(&self) -> &(dyn Bar<u32> + 'a);
}

impl Foo for () {
fn as_dyn_bar_u32<'a>(&self) -> &(dyn Bar<u32> + 'a) {
self
}
}

impl<'a> Deref for dyn Foo + 'a {
type Target = dyn Bar<u32> + 'a;

fn deref(&self) -> &Self::Target {
self.as_dyn_bar_u32()
}
}

fn take_dyn<T>(x: &dyn Bar<T>) -> T {
todo!()
}

fn main() {
let x: &dyn Foo = &();
let y = take_dyn(x);
let z: u32 = y;
//~^ ERROR mismatched types
}
Original file line number Diff line number Diff line change
@@ -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`.
20 changes: 20 additions & 0 deletions tests/ui/traits/trait-upcasting/migrate-lint-different-substs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// check-pass

use std::ops::Deref;

trait Bar<T> {}

trait Foo: Bar<i32> {
fn as_dyn_bar_u32<'a>(&self) -> &(dyn Bar<u32> + 'a);
}

impl<'a> Deref for dyn Foo + 'a {
//~^ WARN this `Deref` implementation is covered by an implicit supertrait coercion
type Target = dyn Bar<u32> + 'a;

fn deref(&self) -> &Self::Target {
self.as_dyn_bar_u32()
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -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<Target = dyn Bar<u32>>` which conflicts with supertrait `Bar<i32>`
LL |
LL | type Target = dyn Bar<u32> + '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