Skip to content

Fix HIR printing of parameters #138482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}))
}

Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(":");
Expand Down
40 changes: 23 additions & 17 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
))
}
Expand Down Expand Up @@ -2710,32 +2710,38 @@ 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,
}
}

fn display(&self, idx: usize) -> impl '_ + fmt::Display {
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)
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/hir/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
38 changes: 38 additions & 0 deletions tests/pretty/hir-fn-params.pp
Original file line number Diff line number Diff line change
@@ -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);
}
34 changes: 34 additions & 0 deletions tests/pretty/hir-fn-params.rs
Original file line number Diff line number Diff line change
@@ -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);
}
2 changes: 1 addition & 1 deletion tests/ui/typeck/cyclic_type_ice.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading