Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit c94cb83

Browse files
committedAug 16, 2023
Auto merge of #112500 - lukas-code:span-ctxt, r=petrochenkov
Fix argument removal suggestion around macros Fixes #112437. Fixes #113866. Helps with #114255. The issue was that `span.find_ancestor_inside(outer)` could previously return a span with a different expansion context from `outer`. This happens for example for the built-in macro `panic!`, which expands to another macro call of `panic_2021!` or `panic_2015!`. Because the call site of `panic_20xx!` has not associated source code, its span currently points to the call site of `panic!` instead. Something similar also happens items that get desugared in AST->HIR lowering. For example, `for` loops get two spans: One "inner" span that has the `.desugaring_kind()` kind set to `DesugaringKind::ForLoop` and one "outer" span that does not. Similar to the macro situation, both of these spans point to the same source code, but have different expansion contexts. This causes problems, because joining two spans with different expansion contexts will usually[^1] not actually join them together to avoid creating "spaghetti" spans that go from the macro definition to the macro call. For example, in the following snippet `full_span` might not actually contain the `adjusted_start` and `adjusted_end`. This caused the broken suggestion / debug ICE in the linked issues. ```rust let adjusted_start = start.find_ancestor_inside(shared_ancestor); let adjusted_end = end.find_ancestor_inside(shared_ancestor); let full_span = adjusted_start.to(adjusted_end) ``` To fix the issue, this PR introduces a new method, `find_ancestor_inside_same_ctxt`, which combines the functionality of `find_ancestor_inside` and `find_ancestor_in_same_ctxt`: It finds an ancestor span that is contained within the parent *and* has the same syntax context, and is therefore safe to extend. This new method should probably be used everywhere, where the returned span is extended, but for now it is just used for the argument removal suggestion. Additionally, this PR fixes a second issue where the function call itself is inside a macro but the arguments come from outside the macro. The test is added in the first commit to include stderr diff, so this is best reviewed commit by commit. [^1]: If one expansion context is the root context and the other is not.
2 parents 1ec628d + 985036b commit c94cb83

File tree

5 files changed

+198
-51
lines changed

5 files changed

+198
-51
lines changed
 

‎compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
519519
// suggestions and labels are (more) correct when an arg is a
520520
// macro invocation.
521521
let normalize_span = |span: Span| -> Span {
522-
let normalized_span = span.find_ancestor_inside(error_span).unwrap_or(span);
522+
let normalized_span = span.find_ancestor_inside_same_ctxt(error_span).unwrap_or(span);
523523
// Sometimes macros mess up the spans, so do not normalize the
524524
// arg span to equal the error span, because that's less useful
525525
// than pointing out the arg expr in the wrong context.
@@ -929,7 +929,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
929929
};
930930
labels.push((provided_span, format!("unexpected argument{provided_ty_name}")));
931931
let mut span = provided_span;
932-
if span.can_be_used_for_suggestions() {
932+
if span.can_be_used_for_suggestions()
933+
&& error_span.can_be_used_for_suggestions()
934+
{
933935
if arg_idx.index() > 0
934936
&& let Some((_, prev)) = provided_arg_tys
935937
.get(ProvidedIdx::from_usize(arg_idx.index() - 1)
@@ -1220,22 +1222,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12201222
};
12211223
if let Some(suggestion_text) = suggestion_text {
12221224
let source_map = self.sess().source_map();
1223-
let (mut suggestion, suggestion_span) =
1224-
if let Some(call_span) = full_call_span.find_ancestor_inside(error_span) {
1225-
("(".to_string(), call_span.shrink_to_hi().to(error_span.shrink_to_hi()))
1226-
} else {
1227-
(
1228-
format!(
1229-
"{}(",
1230-
source_map.span_to_snippet(full_call_span).unwrap_or_else(|_| {
1231-
fn_def_id.map_or("".to_string(), |fn_def_id| {
1232-
tcx.item_name(fn_def_id).to_string()
1233-
})
1225+
let (mut suggestion, suggestion_span) = if let Some(call_span) =
1226+
full_call_span.find_ancestor_inside_same_ctxt(error_span)
1227+
{
1228+
("(".to_string(), call_span.shrink_to_hi().to(error_span.shrink_to_hi()))
1229+
} else {
1230+
(
1231+
format!(
1232+
"{}(",
1233+
source_map.span_to_snippet(full_call_span).unwrap_or_else(|_| {
1234+
fn_def_id.map_or("".to_string(), |fn_def_id| {
1235+
tcx.item_name(fn_def_id).to_string()
12341236
})
1235-
),
1236-
error_span,
1237-
)
1238-
};
1237+
})
1238+
),
1239+
error_span,
1240+
)
1241+
};
12391242
let mut needs_comma = false;
12401243
for (expected_idx, provided_idx) in matched_inputs.iter_enumerated() {
12411244
if needs_comma {

‎compiler/rustc_span/src/lib.rs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -686,18 +686,47 @@ impl Span {
686686
}
687687

688688
/// Walk down the expansion ancestors to find a span that's contained within `outer`.
689+
///
690+
/// The span returned by this method may have a different [`SyntaxContext`] as `outer`.
691+
/// If you need to extend the span, use [`find_ancestor_inside_same_ctxt`] instead,
692+
/// because joining spans with different syntax contexts can create unexpected results.
693+
///
694+
/// [`find_ancestor_inside_same_ctxt`]: Self::find_ancestor_inside_same_ctxt
689695
pub fn find_ancestor_inside(mut self, outer: Span) -> Option<Span> {
690696
while !outer.contains(self) {
691697
self = self.parent_callsite()?;
692698
}
693699
Some(self)
694700
}
695701

696-
/// Like `find_ancestor_inside`, but specifically for when spans might not
697-
/// overlaps. Take care when using this, and prefer `find_ancestor_inside`
698-
/// when you know that the spans are nested (modulo macro expansion).
702+
/// Walk down the expansion ancestors to find a span with the same [`SyntaxContext`] as
703+
/// `other`.
704+
///
705+
/// Like [`find_ancestor_inside_same_ctxt`], but specifically for when spans might not
706+
/// overlap. Take care when using this, and prefer [`find_ancestor_inside`] or
707+
/// [`find_ancestor_inside_same_ctxt`] when you know that the spans are nested (modulo
708+
/// macro expansion).
709+
///
710+
/// [`find_ancestor_inside`]: Self::find_ancestor_inside
711+
/// [`find_ancestor_inside_same_ctxt`]: Self::find_ancestor_inside_same_ctxt
699712
pub fn find_ancestor_in_same_ctxt(mut self, other: Span) -> Option<Span> {
700-
while !Span::eq_ctxt(self, other) {
713+
while !self.eq_ctxt(other) {
714+
self = self.parent_callsite()?;
715+
}
716+
Some(self)
717+
}
718+
719+
/// Walk down the expansion ancestors to find a span that's contained within `outer` and
720+
/// has the same [`SyntaxContext`] as `outer`.
721+
///
722+
/// This method is the combination of [`find_ancestor_inside`] and
723+
/// [`find_ancestor_in_same_ctxt`] and should be preferred when extending the returned span.
724+
/// If you do not need to modify the span, use [`find_ancestor_inside`] instead.
725+
///
726+
/// [`find_ancestor_inside`]: Self::find_ancestor_inside
727+
/// [`find_ancestor_in_same_ctxt`]: Self::find_ancestor_in_same_ctxt
728+
pub fn find_ancestor_inside_same_ctxt(mut self, outer: Span) -> Option<Span> {
729+
while !outer.contains(self) || !self.eq_ctxt(outer) {
701730
self = self.parent_callsite()?;
702731
}
703732
Some(self)

‎tests/ui/argument-suggestions/extra_arguments.rs

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
fn empty() {}
2-
fn one_arg(_a: i32) {}
2+
fn one_arg<T>(_a: T) {}
33
fn two_arg_same(_a: i32, _b: i32) {}
44
fn two_arg_diff(_a: i32, _b: &str) {}
55

66
macro_rules! foo {
7-
($x:expr) => {
7+
($x:expr, ~) => {
88
empty($x, 1); //~ ERROR function takes
9-
}
9+
};
10+
($x:expr, $y:expr) => {
11+
empty($x, $y); //~ ERROR function takes
12+
};
13+
(~, $y:expr) => {
14+
empty(1, $y); //~ ERROR function takes
15+
};
1016
}
1117

1218
fn main() {
@@ -39,5 +45,17 @@ fn main() {
3945
1,
4046
""
4147
);
42-
foo!(1);
48+
49+
// Check with macro expansions
50+
foo!(1, ~);
51+
foo!(~, 1);
52+
foo!(1, 1);
53+
one_arg(1, panic!()); //~ ERROR function takes
54+
one_arg(panic!(), 1); //~ ERROR function takes
55+
one_arg(stringify!($e), 1); //~ ERROR function takes
56+
57+
// Not a macro, but this also has multiple spans with equal source code,
58+
// but different expansion contexts.
59+
// https://github.com/rust-lang/rust/issues/114255
60+
one_arg(for _ in 1.. {}, 1); //~ ERROR function takes
4361
}

‎tests/ui/argument-suggestions/extra_arguments.stderr

Lines changed: 123 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0061]: this function takes 0 arguments but 1 argument was supplied
2-
--> $DIR/extra_arguments.rs:13:3
2+
--> $DIR/extra_arguments.rs:19:3
33
|
44
LL | empty("");
55
| ^^^^^ --
@@ -14,7 +14,7 @@ LL | fn empty() {}
1414
| ^^^^^
1515

1616
error[E0061]: this function takes 0 arguments but 2 arguments were supplied
17-
--> $DIR/extra_arguments.rs:14:3
17+
--> $DIR/extra_arguments.rs:20:3
1818
|
1919
LL | empty(1, 1);
2020
| ^^^^^ - - unexpected argument of type `{integer}`
@@ -33,7 +33,7 @@ LL + empty();
3333
|
3434

3535
error[E0061]: this function takes 1 argument but 2 arguments were supplied
36-
--> $DIR/extra_arguments.rs:16:3
36+
--> $DIR/extra_arguments.rs:22:3
3737
|
3838
LL | one_arg(1, 1);
3939
| ^^^^^^^ ---
@@ -44,11 +44,11 @@ LL | one_arg(1, 1);
4444
note: function defined here
4545
--> $DIR/extra_arguments.rs:2:4
4646
|
47-
LL | fn one_arg(_a: i32) {}
48-
| ^^^^^^^ -------
47+
LL | fn one_arg<T>(_a: T) {}
48+
| ^^^^^^^ -----
4949

5050
error[E0061]: this function takes 1 argument but 2 arguments were supplied
51-
--> $DIR/extra_arguments.rs:17:3
51+
--> $DIR/extra_arguments.rs:23:3
5252
|
5353
LL | one_arg(1, "");
5454
| ^^^^^^^ ----
@@ -59,11 +59,11 @@ LL | one_arg(1, "");
5959
note: function defined here
6060
--> $DIR/extra_arguments.rs:2:4
6161
|
62-
LL | fn one_arg(_a: i32) {}
63-
| ^^^^^^^ -------
62+
LL | fn one_arg<T>(_a: T) {}
63+
| ^^^^^^^ -----
6464

6565
error[E0061]: this function takes 1 argument but 3 arguments were supplied
66-
--> $DIR/extra_arguments.rs:18:3
66+
--> $DIR/extra_arguments.rs:24:3
6767
|
6868
LL | one_arg(1, "", 1.0);
6969
| ^^^^^^^ -- --- unexpected argument of type `{float}`
@@ -73,16 +73,16 @@ LL | one_arg(1, "", 1.0);
7373
note: function defined here
7474
--> $DIR/extra_arguments.rs:2:4
7575
|
76-
LL | fn one_arg(_a: i32) {}
77-
| ^^^^^^^ -------
76+
LL | fn one_arg<T>(_a: T) {}
77+
| ^^^^^^^ -----
7878
help: remove the extra arguments
7979
|
8080
LL - one_arg(1, "", 1.0);
8181
LL + one_arg(1);
8282
|
8383

8484
error[E0061]: this function takes 2 arguments but 3 arguments were supplied
85-
--> $DIR/extra_arguments.rs:20:3
85+
--> $DIR/extra_arguments.rs:26:3
8686
|
8787
LL | two_arg_same(1, 1, 1);
8888
| ^^^^^^^^^^^^ ---
@@ -97,7 +97,7 @@ LL | fn two_arg_same(_a: i32, _b: i32) {}
9797
| ^^^^^^^^^^^^ ------- -------
9898

9999
error[E0061]: this function takes 2 arguments but 3 arguments were supplied
100-
--> $DIR/extra_arguments.rs:21:3
100+
--> $DIR/extra_arguments.rs:27:3
101101
|
102102
LL | two_arg_same(1, 1, 1.0);
103103
| ^^^^^^^^^^^^ -----
@@ -112,7 +112,7 @@ LL | fn two_arg_same(_a: i32, _b: i32) {}
112112
| ^^^^^^^^^^^^ ------- -------
113113

114114
error[E0061]: this function takes 2 arguments but 3 arguments were supplied
115-
--> $DIR/extra_arguments.rs:23:3
115+
--> $DIR/extra_arguments.rs:29:3
116116
|
117117
LL | two_arg_diff(1, 1, "");
118118
| ^^^^^^^^^^^^ ---
@@ -127,7 +127,7 @@ LL | fn two_arg_diff(_a: i32, _b: &str) {}
127127
| ^^^^^^^^^^^^ ------- --------
128128

129129
error[E0061]: this function takes 2 arguments but 3 arguments were supplied
130-
--> $DIR/extra_arguments.rs:24:3
130+
--> $DIR/extra_arguments.rs:30:3
131131
|
132132
LL | two_arg_diff(1, "", "");
133133
| ^^^^^^^^^^^^ ----
@@ -142,7 +142,7 @@ LL | fn two_arg_diff(_a: i32, _b: &str) {}
142142
| ^^^^^^^^^^^^ ------- --------
143143

144144
error[E0061]: this function takes 2 arguments but 4 arguments were supplied
145-
--> $DIR/extra_arguments.rs:25:3
145+
--> $DIR/extra_arguments.rs:31:3
146146
|
147147
LL | two_arg_diff(1, 1, "", "");
148148
| ^^^^^^^^^^^^ - -- unexpected argument of type `&'static str`
@@ -161,7 +161,7 @@ LL + two_arg_diff(1, "");
161161
|
162162

163163
error[E0061]: this function takes 2 arguments but 4 arguments were supplied
164-
--> $DIR/extra_arguments.rs:26:3
164+
--> $DIR/extra_arguments.rs:32:3
165165
|
166166
LL | two_arg_diff(1, "", 1, "");
167167
| ^^^^^^^^^^^^ - -- unexpected argument of type `&'static str`
@@ -180,7 +180,7 @@ LL + two_arg_diff(1, "");
180180
|
181181

182182
error[E0061]: this function takes 2 arguments but 3 arguments were supplied
183-
--> $DIR/extra_arguments.rs:29:3
183+
--> $DIR/extra_arguments.rs:35:3
184184
|
185185
LL | two_arg_same(1, 1, "");
186186
| ^^^^^^^^^^^^ --------
@@ -195,7 +195,7 @@ LL | fn two_arg_same(_a: i32, _b: i32) {}
195195
| ^^^^^^^^^^^^ ------- -------
196196

197197
error[E0061]: this function takes 2 arguments but 3 arguments were supplied
198-
--> $DIR/extra_arguments.rs:30:3
198+
--> $DIR/extra_arguments.rs:36:3
199199
|
200200
LL | two_arg_diff(1, 1, "");
201201
| ^^^^^^^^^^^^ ---
@@ -210,7 +210,7 @@ LL | fn two_arg_diff(_a: i32, _b: &str) {}
210210
| ^^^^^^^^^^^^ ------- --------
211211

212212
error[E0061]: this function takes 2 arguments but 3 arguments were supplied
213-
--> $DIR/extra_arguments.rs:31:3
213+
--> $DIR/extra_arguments.rs:37:3
214214
|
215215
LL | two_arg_same(
216216
| ^^^^^^^^^^^^
@@ -230,7 +230,7 @@ LL | fn two_arg_same(_a: i32, _b: i32) {}
230230
| ^^^^^^^^^^^^ ------- -------
231231

232232
error[E0061]: this function takes 2 arguments but 3 arguments were supplied
233-
--> $DIR/extra_arguments.rs:37:3
233+
--> $DIR/extra_arguments.rs:43:3
234234
|
235235
LL | two_arg_diff(
236236
| ^^^^^^^^^^^^
@@ -254,11 +254,10 @@ error[E0061]: this function takes 0 arguments but 2 arguments were supplied
254254
LL | empty($x, 1);
255255
| ^^^^^ - unexpected argument of type `{integer}`
256256
...
257-
LL | foo!(1);
258-
| -------
257+
LL | foo!(1, ~);
258+
| ----------
259259
| | |
260260
| | unexpected argument of type `{integer}`
261-
| | help: remove the extra argument
262261
| in this macro invocation
263262
|
264263
note: function defined here
@@ -268,6 +267,105 @@ LL | fn empty() {}
268267
| ^^^^^
269268
= note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
270269

271-
error: aborting due to 16 previous errors
270+
error[E0061]: this function takes 0 arguments but 2 arguments were supplied
271+
--> $DIR/extra_arguments.rs:14:9
272+
|
273+
LL | empty(1, $y);
274+
| ^^^^^ - unexpected argument of type `{integer}`
275+
...
276+
LL | foo!(~, 1);
277+
| ----------
278+
| | |
279+
| | unexpected argument of type `{integer}`
280+
| in this macro invocation
281+
|
282+
note: function defined here
283+
--> $DIR/extra_arguments.rs:1:4
284+
|
285+
LL | fn empty() {}
286+
| ^^^^^
287+
= note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
288+
289+
error[E0061]: this function takes 0 arguments but 2 arguments were supplied
290+
--> $DIR/extra_arguments.rs:11:9
291+
|
292+
LL | empty($x, $y);
293+
| ^^^^^
294+
...
295+
LL | foo!(1, 1);
296+
| ----------
297+
| | | |
298+
| | | unexpected argument of type `{integer}`
299+
| | unexpected argument of type `{integer}`
300+
| in this macro invocation
301+
|
302+
note: function defined here
303+
--> $DIR/extra_arguments.rs:1:4
304+
|
305+
LL | fn empty() {}
306+
| ^^^^^
307+
= note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
308+
309+
error[E0061]: this function takes 1 argument but 2 arguments were supplied
310+
--> $DIR/extra_arguments.rs:53:3
311+
|
312+
LL | one_arg(1, panic!());
313+
| ^^^^^^^ ----------
314+
| | |
315+
| | unexpected argument
316+
| help: remove the extra argument
317+
|
318+
note: function defined here
319+
--> $DIR/extra_arguments.rs:2:4
320+
|
321+
LL | fn one_arg<T>(_a: T) {}
322+
| ^^^^^^^ -----
323+
324+
error[E0061]: this function takes 1 argument but 2 arguments were supplied
325+
--> $DIR/extra_arguments.rs:54:3
326+
|
327+
LL | one_arg(panic!(), 1);
328+
| ^^^^^^^ ---
329+
| | |
330+
| | unexpected argument of type `{integer}`
331+
| help: remove the extra argument
332+
|
333+
note: function defined here
334+
--> $DIR/extra_arguments.rs:2:4
335+
|
336+
LL | fn one_arg<T>(_a: T) {}
337+
| ^^^^^^^ -----
338+
339+
error[E0061]: this function takes 1 argument but 2 arguments were supplied
340+
--> $DIR/extra_arguments.rs:55:3
341+
|
342+
LL | one_arg(stringify!($e), 1);
343+
| ^^^^^^^ ---
344+
| | |
345+
| | unexpected argument of type `{integer}`
346+
| help: remove the extra argument
347+
|
348+
note: function defined here
349+
--> $DIR/extra_arguments.rs:2:4
350+
|
351+
LL | fn one_arg<T>(_a: T) {}
352+
| ^^^^^^^ -----
353+
354+
error[E0061]: this function takes 1 argument but 2 arguments were supplied
355+
--> $DIR/extra_arguments.rs:60:3
356+
|
357+
LL | one_arg(for _ in 1.. {}, 1);
358+
| ^^^^^^^ ---
359+
| | |
360+
| | unexpected argument of type `{integer}`
361+
| help: remove the extra argument
362+
|
363+
note: function defined here
364+
--> $DIR/extra_arguments.rs:2:4
365+
|
366+
LL | fn one_arg<T>(_a: T) {}
367+
| ^^^^^^^ -----
368+
369+
error: aborting due to 22 previous errors
272370

273371
For more information about this error, try `rustc --explain E0061`.

‎tests/ui/issues/issue-48364.stderr

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ LL | b"".starts_with(stringify!(foo))
1010
found reference `&'static str`
1111
note: method defined here
1212
--> $SRC_DIR/core/src/slice/mod.rs:LL:COL
13-
= note: this error originates in the macro `stringify` (in Nightly builds, run with -Z macro-backtrace for more info)
1413

1514
error: aborting due to previous error
1615

0 commit comments

Comments
 (0)
Please sign in to comment.