Auto merge of rust-lang#112500 - lukas-code:span-ctxt, r=petrochenkov

Fix argument removal suggestion around macros

Fixes rust-lang#112437.
Fixes rust-lang#113866.
Helps with rust-lang#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.

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -519,7 +519,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         // suggestions and labels are (more) correct when an arg is a
         // macro invocation.
         let normalize_span = |span: Span| -> Span {
-            let normalized_span = span.find_ancestor_inside(error_span).unwrap_or(span);
+            let normalized_span = span.find_ancestor_inside_same_ctxt(error_span).unwrap_or(span);
             // Sometimes macros mess up the spans, so do not normalize the
             // arg span to equal the error span, because that's less useful
             // than pointing out the arg expr in the wrong context.
@@ -929,7 +929,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     };
                     labels.push((provided_span, format!("unexpected argument{provided_ty_name}")));
                     let mut span = provided_span;
-                    if span.can_be_used_for_suggestions() {
+                    if span.can_be_used_for_suggestions()
+                        && error_span.can_be_used_for_suggestions()
+                    {
                         if arg_idx.index() > 0
                         && let Some((_, prev)) = provided_arg_tys
                             .get(ProvidedIdx::from_usize(arg_idx.index() - 1)
@@ -1220,22 +1222,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         };
         if let Some(suggestion_text) = suggestion_text {
             let source_map = self.sess().source_map();
-            let (mut suggestion, suggestion_span) =
-                if let Some(call_span) = full_call_span.find_ancestor_inside(error_span) {
-                    ("(".to_string(), call_span.shrink_to_hi().to(error_span.shrink_to_hi()))
-                } else {
-                    (
-                        format!(
-                            "{}(",
-                            source_map.span_to_snippet(full_call_span).unwrap_or_else(|_| {
-                                fn_def_id.map_or("".to_string(), |fn_def_id| {
-                                    tcx.item_name(fn_def_id).to_string()
-                                })
+            let (mut suggestion, suggestion_span) = if let Some(call_span) =
+                full_call_span.find_ancestor_inside_same_ctxt(error_span)
+            {
+                ("(".to_string(), call_span.shrink_to_hi().to(error_span.shrink_to_hi()))
+            } else {
+                (
+                    format!(
+                        "{}(",
+                        source_map.span_to_snippet(full_call_span).unwrap_or_else(|_| {
+                            fn_def_id.map_or("".to_string(), |fn_def_id| {
+                                tcx.item_name(fn_def_id).to_string()
                             })
-                        ),
-                        error_span,
-                    )
-                };
+                        })
+                    ),
+                    error_span,
+                )
+            };
             let mut needs_comma = false;
             for (expected_idx, provided_idx) in matched_inputs.iter_enumerated() {
                 if needs_comma {

compiler/rustc_span/src/lib.rs

@@ -686,18 +686,47 @@ impl Span {
     }
 
     /// Walk down the expansion ancestors to find a span that's contained within `outer`.
+    ///
+    /// The span returned by this method may have a different [`SyntaxContext`] as `outer`.
+    /// If you need to extend the span, use [`find_ancestor_inside_same_ctxt`] instead,
+    /// because joining spans with different syntax contexts can create unexpected results.
+    ///
+    /// [`find_ancestor_inside_same_ctxt`]: Self::find_ancestor_inside_same_ctxt
     pub fn find_ancestor_inside(mut self, outer: Span) -> Option<Span> {
         while !outer.contains(self) {
             self = self.parent_callsite()?;
         }
         Some(self)
     }
 
-    /// Like `find_ancestor_inside`, but specifically for when spans might not
-    /// overlaps. Take care when using this, and prefer `find_ancestor_inside`
-    /// when you know that the spans are nested (modulo macro expansion).
+    /// Walk down the expansion ancestors to find a span with the same [`SyntaxContext`] as
+    /// `other`.
+    ///
+    /// Like [`find_ancestor_inside_same_ctxt`], but specifically for when spans might not
+    /// overlap. Take care when using this, and prefer [`find_ancestor_inside`] or
+    /// [`find_ancestor_inside_same_ctxt`] when you know that the spans are nested (modulo
+    /// macro expansion).
+    ///
+    /// [`find_ancestor_inside`]: Self::find_ancestor_inside
+    /// [`find_ancestor_inside_same_ctxt`]: Self::find_ancestor_inside_same_ctxt
     pub fn find_ancestor_in_same_ctxt(mut self, other: Span) -> Option<Span> {
-        while !Span::eq_ctxt(self, other) {
+        while !self.eq_ctxt(other) {
+            self = self.parent_callsite()?;
+        }
+        Some(self)
+    }
+
+    /// Walk down the expansion ancestors to find a span that's contained within `outer` and
+    /// has the same [`SyntaxContext`] as `outer`.
+    ///
+    /// This method is the combination of [`find_ancestor_inside`] and
+    /// [`find_ancestor_in_same_ctxt`] and should be preferred when extending the returned span.
+    /// If you do not need to modify the span, use [`find_ancestor_inside`] instead.
+    ///
+    /// [`find_ancestor_inside`]: Self::find_ancestor_inside
+    /// [`find_ancestor_in_same_ctxt`]: Self::find_ancestor_in_same_ctxt
+    pub fn find_ancestor_inside_same_ctxt(mut self, outer: Span) -> Option<Span> {
+        while !outer.contains(self) || !self.eq_ctxt(outer) {
             self = self.parent_callsite()?;
         }
         Some(self)

tests/ui/argument-suggestions/extra_arguments.rs

@@ -1,12 +1,18 @@
 fn empty() {}
-fn one_arg(_a: i32) {}
+fn one_arg<T>(_a: T) {}
 fn two_arg_same(_a: i32, _b: i32) {}
 fn two_arg_diff(_a: i32, _b: &str) {}
 
 macro_rules! foo {
-    ($x:expr) => {
+    ($x:expr, ~) => {
         empty($x, 1); //~ ERROR function takes
-    }
+    };
+    ($x:expr, $y:expr) => {
+        empty($x, $y); //~ ERROR function takes
+    };
+    (~, $y:expr) => {
+        empty(1, $y); //~ ERROR function takes
+    };
 }
 
 fn main() {
@@ -39,5 +45,17 @@ fn main() {
     1,
     ""
   );
-  foo!(1);
+
+  // Check with macro expansions
+  foo!(1, ~);
+  foo!(~, 1);
+  foo!(1, 1);
+  one_arg(1, panic!()); //~ ERROR function takes
+  one_arg(panic!(), 1); //~ ERROR function takes
+  one_arg(stringify!($e), 1); //~ ERROR function takes
+
+  // Not a macro, but this also has multiple spans with equal source code,
+  // but different expansion contexts.
+  // https://github.com/rust-lang/rust/issues/114255
+  one_arg(for _ in 1.. {}, 1); //~ ERROR function takes
 }