Skip to content

Commit 72b4df3

Browse files
compiler-errorscompiler-errors
committed
Implement lint for definition site item shadowing too
1 parent 2189908 commit 72b4df3

15 files changed

+296
-25
lines changed

‎compiler/rustc_hir_analysis/messages.ftl

+6
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,12 @@ hir_analysis_specialization_trait = implementing `rustc_specialization_trait` tr
506506
507507
hir_analysis_static_specialize = cannot specialize on `'static` lifetime
508508
509+
hir_analysis_supertrait_item_multiple_shadowee = items from several supertraits are shadowed: {$traits}
510+
511+
hir_analysis_supertrait_item_shadowee = item from `{$supertrait}` is shadowed by a subtrait item
512+
513+
hir_analysis_supertrait_item_shadowing = trait item `{$item}` from `{$subtrait}` shadows identically named item from supertrait
514+
509515
hir_analysis_tait_forward_compat = item constrains opaque type that is not in its signature
510516
.note = this item must mention the opaque type in its signature in order to be able to register hidden types
511517

‎compiler/rustc_hir_analysis/src/check/wfcheck.rs

+45
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use rustc_hir::lang_items::LangItem;
1212
use rustc_hir::{AmbigArg, ItemKind};
1313
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
1414
use rustc_infer::infer::{self, InferCtxt, TyCtxtInferExt};
15+
use rustc_lint_defs::builtin::SUPERTRAIT_ITEM_SHADOWING_DEFINITION;
1516
use rustc_macros::LintDiagnostic;
1617
use rustc_middle::mir::interpret::ErrorHandled;
1718
use rustc_middle::query::Providers;
@@ -388,7 +389,12 @@ fn check_trait_item<'tcx>(
388389
hir::TraitItemKind::Type(_bounds, Some(ty)) => (None, ty.span),
389390
_ => (None, trait_item.span),
390391
};
392+
391393
check_dyn_incompatible_self_trait_by_name(tcx, trait_item);
394+
395+
// Check that an item definition in a subtrait is shadowing a supertrait item.
396+
lint_item_shadowing_supertrait_item(tcx, def_id);
397+
392398
let mut res = check_associated_item(tcx, def_id, span, method_sig);
393399

394400
if matches!(trait_item.kind, hir::TraitItemKind::Fn(..)) {
@@ -898,6 +904,45 @@ fn check_dyn_incompatible_self_trait_by_name(tcx: TyCtxt<'_>, item: &hir::TraitI
898904
}
899905
}
900906

907+
fn lint_item_shadowing_supertrait_item<'tcx>(tcx: TyCtxt<'tcx>, trait_item_def_id: LocalDefId) {
908+
let item_name = tcx.item_name(trait_item_def_id.to_def_id());
909+
let trait_def_id = tcx.local_parent(trait_item_def_id);
910+
911+
let shadowed: Vec<_> = traits::supertrait_def_ids(tcx, trait_def_id.to_def_id())
912+
.skip(1)
913+
.flat_map(|supertrait_def_id| {
914+
tcx.associated_items(supertrait_def_id).filter_by_name_unhygienic(item_name)
915+
})
916+
.collect();
917+
if !shadowed.is_empty() {
918+
let shadowee = if let [shadowed] = shadowed[..] {
919+
errors::SupertraitItemShadowee::Labeled {
920+
span: tcx.def_span(shadowed.def_id),
921+
supertrait: tcx.item_name(shadowed.trait_container(tcx).unwrap()),
922+
}
923+
} else {
924+
let (traits, spans): (Vec<_>, Vec<_>) = shadowed
925+
.iter()
926+
.map(|item| {
927+
(tcx.item_name(item.trait_container(tcx).unwrap()), tcx.def_span(item.def_id))
928+
})
929+
.unzip();
930+
errors::SupertraitItemShadowee::Several { traits: traits.into(), spans: spans.into() }
931+
};
932+
933+
tcx.emit_node_span_lint(
934+
SUPERTRAIT_ITEM_SHADOWING_DEFINITION,
935+
tcx.local_def_id_to_hir_id(trait_item_def_id),
936+
tcx.def_span(trait_item_def_id),
937+
errors::SupertraitItemShadowing {
938+
item: item_name,
939+
subtrait: tcx.item_name(trait_def_id.to_def_id()),
940+
shadowee,
941+
},
942+
);
943+
}
944+
}
945+
901946
fn check_impl_item<'tcx>(
902947
tcx: TyCtxt<'tcx>,
903948
impl_item: &'tcx hir::ImplItem<'tcx>,

‎compiler/rustc_hir_analysis/src/errors.rs

+27-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
use rustc_abi::ExternAbi;
44
use rustc_errors::codes::*;
55
use rustc_errors::{
6-
Applicability, Diag, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level, MultiSpan,
6+
Applicability, Diag, DiagCtxtHandle, DiagSymbolList, Diagnostic, EmissionGuarantee, Level,
7+
MultiSpan,
78
};
89
use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
910
use rustc_middle::ty::Ty;
@@ -1733,3 +1734,28 @@ pub(crate) struct RegisterTypeUnstable<'a> {
17331734
pub span: Span,
17341735
pub ty: Ty<'a>,
17351736
}
1737+
1738+
#[derive(LintDiagnostic)]
1739+
#[diag(hir_analysis_supertrait_item_shadowing)]
1740+
pub(crate) struct SupertraitItemShadowing {
1741+
pub item: Symbol,
1742+
pub subtrait: Symbol,
1743+
#[subdiagnostic]
1744+
pub shadowee: SupertraitItemShadowee,
1745+
}
1746+
1747+
#[derive(Subdiagnostic)]
1748+
pub(crate) enum SupertraitItemShadowee {
1749+
#[note(hir_analysis_supertrait_item_shadowee)]
1750+
Labeled {
1751+
#[primary_span]
1752+
span: Span,
1753+
supertrait: Symbol,
1754+
},
1755+
#[note(hir_analysis_supertrait_item_multiple_shadowee)]
1756+
Several {
1757+
#[primary_span]
1758+
spans: MultiSpan,
1759+
traits: DiagSymbolList,
1760+
},
1761+
}

‎compiler/rustc_hir_typeck/src/method/confirm.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,10 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
143143
let method_sig = ty::Binder::dummy(method_sig);
144144

145145
// Make sure nobody calls `drop()` explicitly.
146-
self.enforce_illegal_method_limitations(pick);
146+
self.check_for_illegal_method_calls(pick);
147147

148-
self.enforce_shadowed_supertrait_items(pick, segment);
148+
// Lint when an item is shadowing a supertrait item.
149+
self.lint_shadowed_supertrait_items(pick, segment);
149150

150151
// Add any trait/regions obligations specified on the method's type parameters.
151152
// We won't add these if we encountered an illegal sized bound, so that we can use
@@ -660,7 +661,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
660661
})
661662
}
662663

663-
fn enforce_illegal_method_limitations(&self, pick: &probe::Pick<'_>) {
664+
fn check_for_illegal_method_calls(&self, pick: &probe::Pick<'_>) {
664665
// Disallow calls to the method `drop` defined in the `Drop` trait.
665666
if let Some(trait_def_id) = pick.item.trait_container(self.tcx) {
666667
if let Err(e) = callee::check_legal_trait_for_method_call(
@@ -676,7 +677,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
676677
}
677678
}
678679

679-
fn enforce_shadowed_supertrait_items(
680+
fn lint_shadowed_supertrait_items(
680681
&self,
681682
pick: &probe::Pick<'_>,
682683
segment: &hir::PathSegment<'tcx>,

‎compiler/rustc_lint_defs/src/builtin.rs

+41-1
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ declare_lint_pass! {
101101
SINGLE_USE_LIFETIMES,
102102
SOFT_UNSTABLE,
103103
STABLE_FEATURES,
104+
SUPERTRAIT_ITEM_SHADOWING_DEFINITION,
104105
SUPERTRAIT_ITEM_SHADOWING_USAGE,
105106
TAIL_EXPR_DROP_ORDER,
106107
TEST_UNSTABLE_LINT,
@@ -4924,7 +4925,7 @@ declare_lint! {
49244925
///
49254926
/// ### Example
49264927
///
4927-
/// ```rust
4928+
/// ```rust,compile_fail
49284929
/// #![feature(supertrait_item_shadowing)]
49294930
/// #![deny(supertrait_item_shadowing_usage)]
49304931
///
@@ -4959,6 +4960,45 @@ declare_lint! {
49594960
@feature_gate = supertrait_item_shadowing;
49604961
}
49614962

4963+
declare_lint! {
4964+
/// The `supertrait_item_shadowing_definition` lint detects when the
4965+
/// definition of an item that is provided by both a subtrait and
4966+
/// supertrait is shadowed, preferring the subtrait.
4967+
///
4968+
/// ### Example
4969+
///
4970+
/// ```rust,compile_fail
4971+
/// #![feature(supertrait_item_shadowing)]
4972+
/// #![deny(supertrait_item_shadowing_definition)]
4973+
///
4974+
/// trait Upstream {
4975+
/// fn hello(&self) {}
4976+
/// }
4977+
/// impl<T> Upstream for T {}
4978+
///
4979+
/// trait Downstream: Upstream {
4980+
/// fn hello(&self) {}
4981+
/// }
4982+
/// impl<T> Downstream for T {}
4983+
/// ```
4984+
///
4985+
/// {{produces}}
4986+
///
4987+
/// ### Explanation
4988+
///
4989+
/// RFC 3624 specified a heuristic in which a supertrait item would be
4990+
/// shadowed by a subtrait item when ambiguity occurs during item
4991+
/// selection. In order to mitigate side-effects of this happening
4992+
/// silently, this lint detects these cases when users want to deny them
4993+
/// or fix their trait definitions.
4994+
pub SUPERTRAIT_ITEM_SHADOWING_DEFINITION,
4995+
// FIXME(supertrait_item_shadowing): It is not decided if this should
4996+
// warn by default at the usage site.
4997+
Allow,
4998+
"detects when a supertrait item is shadowed by a subtrait item",
4999+
@feature_gate = supertrait_item_shadowing;
5000+
}
5001+
49625002
declare_lint! {
49635003
/// The `ptr_to_integer_transmute_in_consts` lint detects pointer to integer
49645004
/// transmute in const functions and associated constants.

‎tests/ui/methods/supertrait-shadowing/common-ancestor-2.rs

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#![feature(supertrait_item_shadowing)]
55
#![warn(supertrait_item_shadowing_usage)]
6+
#![warn(supertrait_item_shadowing_definition)]
67
#![allow(dead_code)]
78

89
trait A {
@@ -21,6 +22,7 @@ impl<T> B for T {}
2122

2223
trait C: A + B {
2324
fn hello(&self) {
25+
//~^ WARN trait item `hello` from `C` shadows identically named item
2426
println!("C");
2527
}
2628
}

‎tests/ui/methods/supertrait-shadowing/common-ancestor-2.stderr

+24-4
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,36 @@
11
warning: trait item `hello` from `C` shadows identically named item from supertrait
2-
--> $DIR/common-ancestor-2.rs:30:8
2+
--> $DIR/common-ancestor-2.rs:24:5
3+
|
4+
LL | fn hello(&self) {
5+
| ^^^^^^^^^^^^^^^
6+
|
7+
note: items from several supertraits are shadowed: `B` and `A`
8+
--> $DIR/common-ancestor-2.rs:10:5
9+
|
10+
LL | fn hello(&self) {
11+
| ^^^^^^^^^^^^^^^
12+
...
13+
LL | fn hello(&self) {
14+
| ^^^^^^^^^^^^^^^
15+
note: the lint level is defined here
16+
--> $DIR/common-ancestor-2.rs:6:9
17+
|
18+
LL | #![warn(supertrait_item_shadowing_definition)]
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
20+
21+
warning: trait item `hello` from `C` shadows identically named item from supertrait
22+
--> $DIR/common-ancestor-2.rs:32:8
323
|
424
LL | ().hello();
525
| ^^^^^
626
|
727
note: item from `C` shadows a supertrait item
8-
--> $DIR/common-ancestor-2.rs:23:5
28+
--> $DIR/common-ancestor-2.rs:24:5
929
|
1030
LL | fn hello(&self) {
1131
| ^^^^^^^^^^^^^^^
1232
note: items from several supertraits are shadowed: `A` and `B`
13-
--> $DIR/common-ancestor-2.rs:9:5
33+
--> $DIR/common-ancestor-2.rs:10:5
1434
|
1535
LL | fn hello(&self) {
1636
| ^^^^^^^^^^^^^^^
@@ -23,5 +43,5 @@ note: the lint level is defined here
2343
LL | #![warn(supertrait_item_shadowing_usage)]
2444
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2545

26-
warning: 1 warning emitted
46+
warning: 2 warnings emitted
2747

‎tests/ui/methods/supertrait-shadowing/common-ancestor-3.rs

+3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#![feature(supertrait_item_shadowing)]
55
#![warn(supertrait_item_shadowing_usage)]
6+
#![warn(supertrait_item_shadowing_definition)]
67
#![allow(dead_code)]
78

89
trait A {
@@ -21,6 +22,7 @@ impl<T> B for T {}
2122

2223
trait C: A + B {
2324
fn hello(&self) {
25+
//~^ WARN trait item `hello` from `C` shadows identically named item
2426
println!("C");
2527
}
2628
}
@@ -30,6 +32,7 @@ impl<T> C for T {}
3032

3133
trait D: C {
3234
fn hello(&self) {
35+
//~^ WARN trait item `hello` from `D` shadows identically named item
3336
println!("D");
3437
}
3538
}

‎tests/ui/methods/supertrait-shadowing/common-ancestor-3.stderr

+42-4
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,54 @@
1+
warning: trait item `hello` from `C` shadows identically named item from supertrait
2+
--> $DIR/common-ancestor-3.rs:24:5
3+
|
4+
LL | fn hello(&self) {
5+
| ^^^^^^^^^^^^^^^
6+
|
7+
note: items from several supertraits are shadowed: `B` and `A`
8+
--> $DIR/common-ancestor-3.rs:10:5
9+
|
10+
LL | fn hello(&self) {
11+
| ^^^^^^^^^^^^^^^
12+
...
13+
LL | fn hello(&self) {
14+
| ^^^^^^^^^^^^^^^
15+
note: the lint level is defined here
16+
--> $DIR/common-ancestor-3.rs:6:9
17+
|
18+
LL | #![warn(supertrait_item_shadowing_definition)]
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
20+
21+
warning: trait item `hello` from `D` shadows identically named item from supertrait
22+
--> $DIR/common-ancestor-3.rs:34:5
23+
|
24+
LL | fn hello(&self) {
25+
| ^^^^^^^^^^^^^^^
26+
|
27+
note: items from several supertraits are shadowed: `C`, `B`, and `A`
28+
--> $DIR/common-ancestor-3.rs:10:5
29+
|
30+
LL | fn hello(&self) {
31+
| ^^^^^^^^^^^^^^^
32+
...
33+
LL | fn hello(&self) {
34+
| ^^^^^^^^^^^^^^^
35+
...
36+
LL | fn hello(&self) {
37+
| ^^^^^^^^^^^^^^^
38+
139
warning: trait item `hello` from `D` shadows identically named item from supertrait
2-
--> $DIR/common-ancestor-3.rs:39:8
40+
--> $DIR/common-ancestor-3.rs:42:8
341
|
442
LL | ().hello();
543
| ^^^^^
644
|
745
note: item from `D` shadows a supertrait item
8-
--> $DIR/common-ancestor-3.rs:32:5
46+
--> $DIR/common-ancestor-3.rs:34:5
947
|
1048
LL | fn hello(&self) {
1149
| ^^^^^^^^^^^^^^^
1250
note: items from several supertraits are shadowed: `A`, `B`, and `C`
13-
--> $DIR/common-ancestor-3.rs:9:5
51+
--> $DIR/common-ancestor-3.rs:10:5
1452
|
1553
LL | fn hello(&self) {
1654
| ^^^^^^^^^^^^^^^
@@ -26,5 +64,5 @@ note: the lint level is defined here
2664
LL | #![warn(supertrait_item_shadowing_usage)]
2765
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2866

29-
warning: 1 warning emitted
67+
warning: 3 warnings emitted
3068

‎tests/ui/methods/supertrait-shadowing/common-ancestor.rs

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#![feature(supertrait_item_shadowing)]
55
#![warn(supertrait_item_shadowing_usage)]
6+
#![warn(supertrait_item_shadowing_definition)]
67
#![allow(dead_code)]
78

89
trait A {
@@ -14,6 +15,7 @@ impl<T> A for T {}
1415

1516
trait B: A {
1617
fn hello(&self) {
18+
//~^ WARN trait item `hello` from `B` shadows identically named item
1719
println!("B");
1820
}
1921
}

0 commit comments

Comments
 (0)