Rollup merge of #138482 - nnethercote:fix-hir-printing, r=compiler-errors

Fix HIR printing of parameters

HIR pretty printing does the wrong thing for anonymous parameters, and there is no test coverage for it. This PR remedies both of those things.

r? ``@lcnr``

8 files changed, 115 insertions, 24 deletions

diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
index c7d37e2704d..df671cf4b86 100644
--- a/compiler/rustc_ast_lowering/src/lib.rs
+++ b/compiler/rustc_ast_lowering/src/lib.rs
@@ -1516,7 +1516,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> &'hir [Ident] {
         self.arena.alloc_from_iter(decl.inputs.iter().map(|param| match param.pat.kind {
             PatKind::Ident(_, ident, _) => self.lower_ident(ident),
-            _ => Ident::new(kw::Empty, self.lower_span(param.pat.span)),
+            PatKind::Wild => Ident::new(kw::Underscore, self.lower_span(param.pat.span)),
+            _ => {
+                self.dcx().span_delayed_bug(
+                    param.pat.span,
+                    "non-ident/wild param pat must trigger an error",
+                );
+                Ident::new(kw::Empty, self.lower_span(param.pat.span))
+            }
         }))
     }
 
diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs
index 3067766fb4d..1409310339a 100644
--- a/compiler/rustc_hir_pretty/src/lib.rs
+++ b/compiler/rustc_hir_pretty/src/lib.rs
@@ -2148,9 +2148,11 @@ impl<'a> State<'a> {
                 s.print_implicit_self(&decl.implicit_self);
             } else {
                 if let Some(arg_name) = arg_names.get(i) {
-                    s.word(arg_name.to_string());
-                    s.word(":");
-                    s.space();
+                    if arg_name.name != kw::Empty {
+                        s.word(arg_name.to_string());
+                        s.word(":");
+                        s.space();
+                    }
                 } else if let Some(body_id) = body_id {
                     s.ann.nested(s, Nested::BodyParamPat(body_id, i));
                     s.word(":");
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
index b8517701667..96d0a0fc6de 100644
--- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
+++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
@@ -19,7 +19,7 @@ use rustc_middle::ty::visit::TypeVisitableExt;
 use rustc_middle::ty::{self, IsSuggestable, Ty, TyCtxt};
 use rustc_middle::{bug, span_bug};
 use rustc_session::Session;
-use rustc_span::{DUMMY_SP, Ident, Span, Symbol, kw, sym};
+use rustc_span::{DUMMY_SP, Ident, Span, kw, sym};
 use rustc_trait_selection::error_reporting::infer::{FailureCode, ObligationCauseExt};
 use rustc_trait_selection::infer::InferCtxtExt;
 use rustc_trait_selection::traits::{self, ObligationCauseCode, ObligationCtxt, SelectionContext};
@@ -2679,7 +2679,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     params.get(is_method as usize..params.len() - sig.decl.c_variadic as usize)?;
                 debug_assert_eq!(params.len(), fn_inputs.len());
                 Some((
-                    fn_inputs.zip(params.iter().map(|param| FnParam::Name(param))).collect(),
+                    fn_inputs.zip(params.iter().map(|&param| FnParam::Name(param))).collect(),
                     generics,
                 ))
             }
@@ -2710,23 +2710,14 @@ impl<'tcx> Visitor<'tcx> for FindClosureArg<'tcx> {
 #[derive(Clone, Copy)]
 enum FnParam<'hir> {
     Param(&'hir hir::Param<'hir>),
-    Name(&'hir Ident),
+    Name(Ident),
 }
+
 impl FnParam<'_> {
     fn span(&self) -> Span {
         match self {
-            Self::Param(x) => x.span,
-            Self::Name(x) => x.span,
-        }
-    }
-
-    fn name(&self) -> Option<Symbol> {
-        match self {
-            Self::Param(x) if let hir::PatKind::Binding(_, _, ident, _) = x.pat.kind => {
-                Some(ident.name)
-            }
-            Self::Name(x) if x.name != kw::Empty => Some(x.name),
-            _ => None,
+            Self::Param(param) => param.span,
+            Self::Name(ident) => ident.span,
         }
     }
 
@@ -2734,8 +2725,23 @@ impl FnParam<'_> {
         struct D<'a>(FnParam<'a>, usize);
         impl fmt::Display for D<'_> {
             fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-                if let Some(name) = self.0.name() {
-                    write!(f, "`{name}`")
+                // A "unique" param name is one that (a) exists, and (b) is guaranteed to be unique
+                // among the parameters, i.e. `_` does not count.
+                let unique_name = match self.0 {
+                    FnParam::Param(param)
+                        if let hir::PatKind::Binding(_, _, ident, _) = param.pat.kind =>
+                    {
+                        Some(ident.name)
+                    }
+                    FnParam::Name(ident)
+                        if ident.name != kw::Empty && ident.name != kw::Underscore =>
+                    {
+                        Some(ident.name)
+                    }
+                    _ => None,
+                };
+                if let Some(unique_name) = unique_name {
+                    write!(f, "`{unique_name}`")
                 } else {
                     write!(f, "parameter #{}", self.1 + 1)
                 }
diff --git a/compiler/rustc_middle/src/hir/map.rs b/compiler/rustc_middle/src/hir/map.rs
index 73c0af84a9f..c61c7a4fb02 100644
--- a/compiler/rustc_middle/src/hir/map.rs
+++ b/compiler/rustc_middle/src/hir/map.rs
@@ -281,8 +281,9 @@ impl<'tcx> TyCtxt<'tcx> {
     }
 
     pub fn hir_body_param_names(self, id: BodyId) -> impl Iterator<Item = Ident> {
-        self.hir_body(id).params.iter().map(|arg| match arg.pat.kind {
+        self.hir_body(id).params.iter().map(|param| match param.pat.kind {
             PatKind::Binding(_, _, ident, _) => ident,
+            PatKind::Wild => Ident::new(kw::Underscore, param.pat.span),
             _ => Ident::empty(),
         })
     }
diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
index dce85b43df1..9383b82ff3c 100644
--- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
+++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
@@ -1998,7 +1998,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                 .iter()
                 .enumerate()
                 .map(|(i, ident)| {
-                    if ident.name.is_empty() || ident.name == kw::SelfLower {
+                    if ident.name.is_empty()
+                        || ident.name == kw::Underscore
+                        || ident.name == kw::SelfLower
+                    {
                         format!("arg{i}")
                     } else {
                         format!("{ident}")
diff --git a/tests/pretty/hir-fn-params.pp b/tests/pretty/hir-fn-params.pp
new file mode 100644
index 00000000000..3799c8a3c3b
--- /dev/null
+++ b/tests/pretty/hir-fn-params.pp
@@ -0,0 +1,38 @@
+#[prelude_import]
+use ::std::prelude::rust_2015::*;
+#[macro_use]
+extern crate std;
+//@ pretty-compare-only
+//@ pretty-mode:hir
+//@ pp-exact:hir-fn-params.pp
+
+// This tests the pretty-printing of various kinds of function parameters.
+
+//---------------------------------------------------------------------------
+// Normal functions and methods.
+
+fn normal_fn(_: u32, a: u32) { }
+
+struct S;
+impl S {
+    fn method(_: u32, a: u32) { }
+}
+
+//---------------------------------------------------------------------------
+// More exotic forms, which get a different pretty-printing path. In the past,
+// anonymous params and `_` params printed incorrectly, e.g. `fn(u32, _: u32)`
+// was printed as `fn(: u32, : u32)`.
+//
+// Ideally we would also test invalid patterns, e.g. `fn(1: u32, &a: u32)`,
+// because they had similar problems. But the pretty-printing tests currently
+// can't contain compile errors.
+
+fn bare_fn(x: fn(u32, _: u32, a: u32)) { }
+
+extern "C" {
+    unsafe fn foreign_fn(_: u32, a: u32);
+}
+
+trait T {
+    fn trait_fn(u32, _: u32, a: u32);
+}
diff --git a/tests/pretty/hir-fn-params.rs b/tests/pretty/hir-fn-params.rs
new file mode 100644
index 00000000000..5ace5289d08
--- /dev/null
+++ b/tests/pretty/hir-fn-params.rs
@@ -0,0 +1,34 @@
+//@ pretty-compare-only
+//@ pretty-mode:hir
+//@ pp-exact:hir-fn-params.pp
+
+// This tests the pretty-printing of various kinds of function parameters.
+
+//---------------------------------------------------------------------------
+// Normal functions and methods.
+
+fn normal_fn(_: u32, a: u32) {}
+
+struct S;
+impl S {
+    fn method(_: u32, a: u32) {}
+}
+
+//---------------------------------------------------------------------------
+// More exotic forms, which get a different pretty-printing path. In the past,
+// anonymous params and `_` params printed incorrectly, e.g. `fn(u32, _: u32)`
+// was printed as `fn(: u32, : u32)`.
+//
+// Ideally we would also test invalid patterns, e.g. `fn(1: u32, &a: u32)`,
+// because they had similar problems. But the pretty-printing tests currently
+// can't contain compile errors.
+
+fn bare_fn(x: fn(u32, _: u32, a: u32)) {}
+
+extern "C" {
+    fn foreign_fn(_: u32, a: u32);
+}
+
+trait T {
+    fn trait_fn(u32, _: u32, a: u32);
+}
diff --git a/tests/ui/typeck/cyclic_type_ice.stderr b/tests/ui/typeck/cyclic_type_ice.stderr
index 4dc02a53c02..645766becbf 100644
--- a/tests/ui/typeck/cyclic_type_ice.stderr
+++ b/tests/ui/typeck/cyclic_type_ice.stderr
@@ -23,7 +23,7 @@ LL |     let f = |_, _| ();
 help: provide the argument
    |
 LL -     f(f);
-LL +     f(/*  */, /*  */);
+LL +     f(/* _ */, /* _ */);
    |
 
 error: aborting due to 2 previous errors