Skip to content

Commit 8409aaa

Browse files
compiler-errorscompiler-errors
committed
Implement lint for definition site item shadowing too
1 parent f406879 commit 8409aaa

14 files changed

+288
-20
lines changed

‎compiler/rustc_hir_analysis/messages.ftl

+6
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,12 @@ hir_analysis_specialization_trait = implementing `rustc_specialization_trait` tr
497497
498498
hir_analysis_static_specialize = cannot specialize on `'static` lifetime
499499
500+
hir_analysis_supertrait_item_multiple_shadowee = items from several supertraits are shadowed: {$traits}
501+
502+
hir_analysis_supertrait_item_shadowee = item from `{$supertrait}` is shadowed by a subtrait item
503+
504+
hir_analysis_supertrait_item_shadowing = trait item `{$item}` from `{$subtrait}` shadows identically named item from supertrait
505+
500506
hir_analysis_tait_forward_compat = item constrains opaque type that is not in its signature
501507
.note = this item must mention the opaque type in its signature in order to be able to register hidden types
502508

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

+43
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;
@@ -377,7 +378,10 @@ fn check_trait_item<'tcx>(
377378
hir::TraitItemKind::Type(_bounds, Some(ty)) => (None, ty.span),
378379
_ => (None, trait_item.span),
379380
};
381+
380382
check_dyn_incompatible_self_trait_by_name(tcx, trait_item);
383+
check_item_shadowed_by_supertrait(tcx, def_id);
384+
381385
let mut res = check_associated_item(tcx, def_id, span, method_sig);
382386

383387
if matches!(trait_item.kind, hir::TraitItemKind::Fn(..)) {
@@ -892,6 +896,45 @@ fn check_dyn_incompatible_self_trait_by_name(tcx: TyCtxt<'_>, item: &hir::TraitI
892896
}
893897
}
894898

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

‎compiler/rustc_hir_analysis/src/errors.rs

+27-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
33
use rustc_errors::codes::*;
44
use rustc_errors::{
5-
Applicability, Diag, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level, MultiSpan,
5+
Applicability, Diag, DiagCtxtHandle, DiagSymbolList, Diagnostic, EmissionGuarantee, Level,
6+
MultiSpan,
67
};
78
use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
89
use rustc_middle::ty::Ty;
@@ -1702,3 +1703,28 @@ pub(crate) struct RegisterTypeUnstable<'a> {
17021703
pub span: Span,
17031704
pub ty: Ty<'a>,
17041705
}
1706+
1707+
#[derive(LintDiagnostic)]
1708+
#[diag(hir_analysis_supertrait_item_shadowing)]
1709+
pub(crate) struct SupertraitItemShadowing {
1710+
pub item: Symbol,
1711+
pub subtrait: Symbol,
1712+
#[subdiagnostic]
1713+
pub shadowee: SupertraitItemShadowee,
1714+
}
1715+
1716+
#[derive(Subdiagnostic)]
1717+
pub(crate) enum SupertraitItemShadowee {
1718+
#[note(hir_analysis_supertrait_item_shadowee)]
1719+
Labeled {
1720+
#[primary_span]
1721+
span: Span,
1722+
supertrait: Symbol,
1723+
},
1724+
#[note(hir_analysis_supertrait_item_multiple_shadowee)]
1725+
Several {
1726+
#[primary_span]
1727+
spans: MultiSpan,
1728+
traits: DiagSymbolList,
1729+
},
1730+
}

‎compiler/rustc_lint_defs/src/builtin.rs

+40
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,
@@ -5009,6 +5010,45 @@ declare_lint! {
50095010
@feature_gate = supertrait_item_shadowing;
50105011
}
50115012

5013+
declare_lint! {
5014+
/// The `supertrait_item_shadowing_usage` lint detects when the
5015+
/// definition of an item that is provided by both a subtrait and
5016+
/// supertrait is shadowed, preferring the subtrait.
5017+
///
5018+
/// ### Example
5019+
///
5020+
/// ```rust
5021+
/// #![feature(supertrait_item_shadowing)]
5022+
/// #![deny(supertrait_item_shadowing_definition)]
5023+
///
5024+
/// trait Upstream {
5025+
/// fn hello(&self) {}
5026+
/// }
5027+
/// impl<T> Upstream for T {}
5028+
///
5029+
/// trait Downstream: Upstream {
5030+
/// fn hello(&self) {}
5031+
/// }
5032+
/// impl<T> Downstream for T {}
5033+
/// ```
5034+
///
5035+
/// {{produces}}
5036+
///
5037+
/// ### Explanation
5038+
///
5039+
/// RFC 3624 specified a heuristic in which a supertrait item would be
5040+
/// shadowed by a subtrait item when ambiguity occurs during item
5041+
/// selection. In order to mitigate side-effects of this happening
5042+
/// silently, this lint detects these cases when users want to deny them
5043+
/// or fix their trait definitions.
5044+
pub SUPERTRAIT_ITEM_SHADOWING_DEFINITION,
5045+
// FIXME(supertrait_item_shadowing): It is not decided if this should
5046+
// warn by default at the usage site.
5047+
Allow,
5048+
"detects when a supertrait item is shadowed by a subtrait item",
5049+
@feature_gate = supertrait_item_shadowing;
5050+
}
5051+
50125052
declare_lint! {
50135053
/// The `ptr_to_integer_transmute_in_consts` lint detects pointer to integer
50145054
/// 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
}

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

+21-4
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,33 @@
11
warning: trait item `hello` from `B` shadows identically named item from supertrait
2-
--> $DIR/common-ancestor.rs:23:8
2+
--> $DIR/common-ancestor.rs:17:5
3+
|
4+
LL | fn hello(&self) {
5+
| ^^^^^^^^^^^^^^^
6+
|
7+
note: item from `A` is shadowed by a subtrait item
8+
--> $DIR/common-ancestor.rs:10:5
9+
|
10+
LL | fn hello(&self) {
11+
| ^^^^^^^^^^^^^^^
12+
note: the lint level is defined here
13+
--> $DIR/common-ancestor.rs:6:9
14+
|
15+
LL | #![warn(supertrait_item_shadowing_definition)]
16+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
17+
18+
warning: trait item `hello` from `B` shadows identically named item from supertrait
19+
--> $DIR/common-ancestor.rs:25:8
320
|
421
LL | ().hello();
522
| ^^^^^
623
|
724
note: item from `B` shadows a supertrait item
8-
--> $DIR/common-ancestor.rs:16:5
25+
--> $DIR/common-ancestor.rs:17:5
926
|
1027
LL | fn hello(&self) {
1128
| ^^^^^^^^^^^^^^^
1229
note: item from `A` is shadowed by a subtrait item
13-
--> $DIR/common-ancestor.rs:9:5
30+
--> $DIR/common-ancestor.rs:10:5
1431
|
1532
LL | fn hello(&self) {
1633
| ^^^^^^^^^^^^^^^
@@ -20,5 +37,5 @@ note: the lint level is defined here
2037
LL | #![warn(supertrait_item_shadowing_usage)]
2138
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2239

23-
warning: 1 warning emitted
40+
warning: 2 warnings emitted
2441

0 commit comments

Comments
 (0)