Skip to content

Commit d3dfe14

Browse files
UrgauUrgau
committed
non_local_defs: be more precise about what needs to be moved
1 parent 402580b commit d3dfe14

13 files changed

+171
-232
lines changed

‎compiler/rustc_lint/src/lints.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,8 +1337,7 @@ pub enum NonLocalDefinitionsDiag {
13371337
cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
13381338
const_anon: Option<Option<Span>>,
13391339
move_help: Span,
1340-
self_ty: Span,
1341-
of_trait: Option<Span>,
1340+
may_move: Vec<Span>,
13421341
has_trait: bool,
13431342
},
13441343
MacroRules {
@@ -1361,8 +1360,7 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
13611360
cargo_update,
13621361
const_anon,
13631362
move_help,
1364-
self_ty,
1365-
of_trait,
1363+
may_move,
13661364
has_trait,
13671365
} => {
13681366
diag.primary_message(fluent::lint_non_local_definitions_impl);
@@ -1376,10 +1374,10 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
13761374
} else {
13771375
diag.note(fluent::lint_without_trait);
13781376
}
1377+
13791378
let mut ms = MultiSpan::from_span(move_help);
1380-
ms.push_span_label(self_ty, fluent::lint_non_local_definitions_may_move);
1381-
if let Some(of_trait) = of_trait {
1382-
ms.push_span_label(of_trait, fluent::lint_non_local_definitions_may_move);
1379+
for sp in may_move {
1380+
ms.push_span_label(sp, fluent::lint_non_local_definitions_may_move);
13831381
}
13841382
diag.span_help(ms, fluent::lint_help);
13851383

‎compiler/rustc_lint/src/non_local_def.rs

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use rustc_hir::intravisit::{self, Visitor};
2+
use rustc_hir::HirId;
13
use rustc_hir::{def::DefKind, Body, Item, ItemKind, Node, TyKind};
24
use rustc_hir::{Path, QPath};
35
use rustc_infer::infer::InferCtxt;
@@ -214,6 +216,29 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
214216
None
215217
};
216218

219+
let mut collector = PathCollector { paths: Vec::new() };
220+
collector.visit_ty(&impl_.self_ty);
221+
if let Some(of_trait) = &impl_.of_trait {
222+
collector.visit_trait_ref(of_trait);
223+
}
224+
collector.visit_generics(&impl_.generics);
225+
226+
let may_move: Vec<Span> = collector
227+
.paths
228+
.into_iter()
229+
.filter_map(|path| {
230+
if path_has_local_parent(&path, cx, parent, parent_parent) {
231+
if let Some(args) = &path.segments.last().unwrap().args {
232+
Some(path.span.until(args.span_ext))
233+
} else {
234+
Some(path.span)
235+
}
236+
} else {
237+
None
238+
}
239+
})
240+
.collect();
241+
217242
let const_anon = matches!(parent_def_kind, DefKind::Const | DefKind::Static { .. })
218243
.then_some(span_for_const_anon_suggestion);
219244

@@ -223,14 +248,13 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
223248
NonLocalDefinitionsDiag::Impl {
224249
depth: self.body_depth,
225250
move_help: item.span,
226-
self_ty: impl_.self_ty.span,
227-
of_trait: impl_.of_trait.map(|t| t.path.span),
228251
body_kind_descr: cx.tcx.def_kind_descr(parent_def_kind, parent),
229252
body_name: parent_opt_item_name
230253
.map(|s| s.to_ident_string())
231254
.unwrap_or_else(|| "<unnameable>".to_string()),
232255
cargo_update: cargo_update(),
233256
const_anon,
257+
may_move,
234258
has_trait: impl_.of_trait.is_some(),
235259
},
236260
)
@@ -348,6 +372,18 @@ impl<'a, 'tcx, F: FnMut(DefId) -> bool> TypeFolder<TyCtxt<'tcx>>
348372
}
349373
}
350374

375+
/// Simple hir::Path collector
376+
struct PathCollector<'tcx> {
377+
paths: Vec<Path<'tcx>>,
378+
}
379+
380+
impl<'tcx> Visitor<'tcx> for PathCollector<'tcx> {
381+
fn visit_path(&mut self, path: &Path<'tcx>, _id: HirId) {
382+
self.paths.push(path.clone()); // need to clone, bc of the restricted lifetime
383+
intravisit::walk_path(self, path)
384+
}
385+
}
386+
351387
/// Given a path and a parent impl def id, this checks if the if parent resolution
352388
/// def id correspond to the def id of the parent impl definition.
353389
///

‎tests/ui/lint/non-local-defs/cargo-update.stderr

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@ help: move this `impl` block outside of the current constant `_IMPL_DEBUG`
1111
|
1212
LL | non_local_macro::non_local_impl!(LocalStruct);
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14-
| |
15-
| may need to be moved as well
16-
| may need to be moved as well
1714
= note: the macro `non_local_macro::non_local_impl` may come from an old version of the `non_local_macro` crate, try updating your dependency with `cargo update -p non_local_macro`
1815
= note: items in an anonymous const item (`const _: () = { ... }`) are treated as in the same scope as the anonymous const's declaration
1916
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>

‎tests/ui/lint/non-local-defs/consts.stderr

Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ help: move this `impl` block outside of the current constant `Z`
1313
--> $DIR/consts.rs:13:5
1414
|
1515
LL | impl Uto for &Test {}
16-
| ^^^^^---^^^^^-----^^^
17-
| | |
18-
| | may need to be moved as well
19-
| may need to be moved as well
16+
| ^^^^^^^^^^^^^^^^^^^^^
2017
= note: items in an anonymous const item (`const _: () = { ... }`) are treated as in the same scope as the anonymous const's declaration
2118
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
2219
= note: `#[warn(non_local_definitions)]` on by default
@@ -33,10 +30,7 @@ help: move this `impl` block outside of the current static `A`
3330
--> $DIR/consts.rs:24:5
3431
|
3532
LL | impl Uto2 for Test {}
36-
| ^^^^^----^^^^^----^^^
37-
| | |
38-
| | may need to be moved as well
39-
| may need to be moved as well
33+
| ^^^^^^^^^^^^^^^^^^^^^
4034
= note: items in an anonymous const item (`const _: () = { ... }`) are treated as in the same scope as the anonymous const's declaration
4135
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
4236

@@ -52,10 +46,7 @@ help: move this `impl` block outside of the current constant `B`
5246
--> $DIR/consts.rs:32:5
5347
|
5448
LL | impl Uto3 for Test {}
55-
| ^^^^^----^^^^^----^^^
56-
| | |
57-
| | may need to be moved as well
58-
| may need to be moved as well
49+
| ^^^^^^^^^^^^^^^^^^^^^
5950
= note: items in an anonymous const item (`const _: () = { ... }`) are treated as in the same scope as the anonymous const's declaration
6051
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
6152

@@ -69,10 +60,7 @@ LL | impl Test {
6960
help: move this `impl` block outside of the current function `main`
7061
--> $DIR/consts.rs:43:5
7162
|
72-
LL | impl Test {
73-
| ^ ---- may need to be moved as well
74-
| _____|
75-
| |
63+
LL | / impl Test {
7664
LL | |
7765
LL | | fn foo() {}
7866
LL | | }
@@ -89,10 +77,7 @@ LL | impl Test {
8977
help: move this `impl` block outside of the current inline constant `<unnameable>` and up 2 bodies
9078
--> $DIR/consts.rs:50:9
9179
|
92-
LL | impl Test {
93-
| ^ ---- may need to be moved as well
94-
| _________|
95-
| |
80+
LL | / impl Test {
9681
LL | |
9782
LL | | fn hoo() {}
9883
LL | | }
@@ -109,10 +94,7 @@ LL | impl Test {
10994
help: move this `impl` block outside of the current constant `_` and up 2 bodies
11095
--> $DIR/consts.rs:59:9
11196
|
112-
LL | impl Test {
113-
| ^ ---- may need to be moved as well
114-
| _________|
115-
| |
97+
LL | / impl Test {
11698
LL | |
11799
LL | | fn foo2() {}
118100
LL | | }
@@ -132,10 +114,7 @@ help: move this `impl` block outside of the current closure `<unnameable>` and u
132114
--> $DIR/consts.rs:72:9
133115
|
134116
LL | impl Uto9 for Test {}
135-
| ^^^^^----^^^^^----^^^
136-
| | |
137-
| | may need to be moved as well
138-
| may need to be moved as well
117+
| ^^^^^^^^^^^^^^^^^^^^^
139118
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
140119

141120
warning: non-local `impl` definition, `impl` blocks should be written at the same level as their item
@@ -150,10 +129,7 @@ help: move this `impl` block outside of the current constant expression `<unname
150129
--> $DIR/consts.rs:79:9
151130
|
152131
LL | impl Uto10 for Test {}
153-
| ^^^^^-----^^^^^----^^^
154-
| | |
155-
| | may need to be moved as well
156-
| may need to be moved as well
132+
| ^^^^^^^^^^^^^^^^^^^^^^
157133
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
158134

159135
warning: 8 warnings emitted

‎tests/ui/lint/non-local-defs/exhaustive-trait.stderr

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,7 @@ LL | impl PartialEq<()> for Dog {
99
help: move this `impl` block outside of the current function `main`
1010
--> $DIR/exhaustive-trait.rs:7:5
1111
|
12-
LL | impl PartialEq<()> for Dog {
13-
| ^ ------------- --- may need to be moved as well
14-
| | |
15-
| _____| may need to be moved as well
16-
| |
12+
LL | / impl PartialEq<()> for Dog {
1713
LL | |
1814
LL | | fn eq(&self, _: &()) -> bool {
1915
LL | | todo!()
@@ -34,11 +30,7 @@ LL | impl PartialEq<()> for &Dog {
3430
help: move this `impl` block outside of the current function `main`
3531
--> $DIR/exhaustive-trait.rs:14:5
3632
|
37-
LL | impl PartialEq<()> for &Dog {
38-
| ^ ------------- ---- may need to be moved as well
39-
| | |
40-
| _____| may need to be moved as well
41-
| |
33+
LL | / impl PartialEq<()> for &Dog {
4234
LL | |
4335
LL | | fn eq(&self, _: &()) -> bool {
4436
LL | | todo!()
@@ -58,11 +50,7 @@ LL | impl PartialEq<Dog> for () {
5850
help: move this `impl` block outside of the current function `main`
5951
--> $DIR/exhaustive-trait.rs:21:5
6052
|
61-
LL | impl PartialEq<Dog> for () {
62-
| ^ -------------- -- may need to be moved as well
63-
| | |
64-
| _____| may need to be moved as well
65-
| |
53+
LL | / impl PartialEq<Dog> for () {
6654
LL | |
6755
LL | | fn eq(&self, _: &Dog) -> bool {
6856
LL | | todo!()
@@ -82,11 +70,7 @@ LL | impl PartialEq<&Dog> for () {
8270
help: move this `impl` block outside of the current function `main`
8371
--> $DIR/exhaustive-trait.rs:28:5
8472
|
85-
LL | impl PartialEq<&Dog> for () {
86-
| ^ --------------- -- may need to be moved as well
87-
| | |
88-
| _____| may need to be moved as well
89-
| |
73+
LL | / impl PartialEq<&Dog> for () {
9074
LL | |
9175
LL | | fn eq(&self, _: &&Dog) -> bool {
9276
LL | | todo!()
@@ -106,11 +90,7 @@ LL | impl PartialEq<Dog> for &Dog {
10690
help: move this `impl` block outside of the current function `main`
10791
--> $DIR/exhaustive-trait.rs:35:5
10892
|
109-
LL | impl PartialEq<Dog> for &Dog {
110-
| ^ -------------- ---- may need to be moved as well
111-
| | |
112-
| _____| may need to be moved as well
113-
| |
93+
LL | / impl PartialEq<Dog> for &Dog {
11494
LL | |
11595
LL | | fn eq(&self, _: &Dog) -> bool {
11696
LL | | todo!()
@@ -130,11 +110,7 @@ LL | impl PartialEq<&Dog> for &Dog {
130110
help: move this `impl` block outside of the current function `main`
131111
--> $DIR/exhaustive-trait.rs:42:5
132112
|
133-
LL | impl PartialEq<&Dog> for &Dog {
134-
| ^ --------------- ---- may need to be moved as well
135-
| | |
136-
| _____| may need to be moved as well
137-
| |
113+
LL | / impl PartialEq<&Dog> for &Dog {
138114
LL | |
139115
LL | | fn eq(&self, _: &&Dog) -> bool {
140116
LL | | todo!()

0 commit comments

Comments
 (0)