Skip to content

Commit 88e61fe

Browse files
UrgauUrgau
committed
Be more precise about what needs to be moved
1 parent 14313df commit 88e61fe

13 files changed

+156
-232
lines changed

‎compiler/rustc_lint/src/lints.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1341,8 +1341,7 @@ pub enum NonLocalDefinitionsDiag {
13411341
cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
13421342
const_anon: Option<Option<Span>>,
13431343
move_help: Span,
1344-
self_ty: Span,
1345-
of_trait: Option<Span>,
1344+
may_move: Vec<Span>,
13461345
has_trait: bool,
13471346
},
13481347
MacroRules {
@@ -1365,8 +1364,7 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
13651364
cargo_update,
13661365
const_anon,
13671366
move_help,
1368-
self_ty,
1369-
of_trait,
1367+
may_move,
13701368
has_trait,
13711369
} => {
13721370
diag.arg("depth", depth);
@@ -1379,10 +1377,10 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
13791377
} else {
13801378
diag.note(fluent::lint_without_trait);
13811379
}
1380+
13821381
let mut ms = MultiSpan::from_span(move_help);
1383-
ms.push_span_label(self_ty, fluent::lint_non_local_definitions_may_move);
1384-
if let Some(of_trait) = of_trait {
1385-
ms.push_span_label(of_trait, fluent::lint_non_local_definitions_may_move);
1382+
for sp in may_move {
1383+
ms.push_span_label(sp, fluent::lint_non_local_definitions_may_move);
13861384
}
13871385
diag.span_help(ms, fluent::lint_help);
13881386

‎compiler/rustc_lint/src/non_local_def.rs

Lines changed: 37 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,28 @@ 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+
225+
let may_move: Vec<Span> = collector
226+
.paths
227+
.into_iter()
228+
.filter_map(|path| {
229+
if path_has_local_parent(&path, cx, parent, parent_parent) {
230+
if let Some(args) = &path.segments.last().unwrap().args {
231+
Some(path.span.until(args.span_ext))
232+
} else {
233+
Some(path.span)
234+
}
235+
} else {
236+
None
237+
}
238+
})
239+
.collect();
240+
217241
let const_anon = matches!(parent_def_kind, DefKind::Const | DefKind::Static { .. })
218242
.then_some(span_for_const_anon_suggestion);
219243

@@ -223,14 +247,13 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
223247
NonLocalDefinitionsDiag::Impl {
224248
depth: self.body_depth,
225249
move_help: item.span,
226-
self_ty: impl_.self_ty.span,
227-
of_trait: impl_.of_trait.map(|t| t.path.span),
228250
body_kind_descr: cx.tcx.def_kind_descr(parent_def_kind, parent),
229251
body_name: parent_opt_item_name
230252
.map(|s| s.to_ident_string())
231253
.unwrap_or_else(|| "<unnameable>".to_string()),
232254
cargo_update: cargo_update(),
233255
const_anon,
256+
may_move,
234257
has_trait: impl_.of_trait.is_some(),
235258
},
236259
)
@@ -348,6 +371,18 @@ impl<'a, 'tcx, F: FnMut(DefId) -> bool> TypeFolder<TyCtxt<'tcx>>
348371
}
349372
}
350373

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

‎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)