bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Use Option<Ident> for lowered param names.

Browse source 
Parameter patterns are lowered to an `Ident` by
`lower_fn_params_to_names`, which is used when lowering bare function
types, trait methods, and foreign functions. Currently, there are two
exceptional cases where the lowered param can become an empty `Ident`.

- If the incoming pattern is an empty `Ident`. This occurs if the
  parameter is anonymous, e.g. in a bare function type.

- If the incoming pattern is neither an ident nor an underscore. Any
  such parameter will have triggered a compile error (hence the
  `span_delayed_bug`), but lowering still occurs.

This commit replaces these empty `Ident` results with `None`, which
eliminates a number of `kw::Empty` uses, and makes it impossible to fail
to check for these exceptional cases.

Note: the `FIXME` comment in `is_unwrap_or_empty_symbol` is removed. It
actually should have been removed in #138482, the precursor to this PR.
That PR changed the lowering of wild patterns to `_` symbols instead of
empty symbols, which made the mentioned underscore check load-bearing.
This commit is contained in:
Nicholas Nethercote 2025-03-14 09:03:23 +11:00
parent 75530e9f72
commit f27cab806e
20 changed files with 125 additions and 87 deletions
Download patch file Download diff file Expand all files Collapse all files

14
compiler/rustc_ast_lowering/src/lib.rs
View file

@ -1513,16 +1513,22 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}))
}
fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> &'hir [Ident] {
fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> &'hir [Option<Ident>] {
self.arena.alloc_from_iter(decl.inputs.iter().map(|param| match param.pat.kind {
PatKind::Ident(_, ident, _) => self.lower_ident(ident),
PatKind::Wild => Ident::new(kw::Underscore, self.lower_span(param.pat.span)),
PatKind::Ident(_, ident, _) => {
if ident.name != kw::Empty {
Some(self.lower_ident(ident))
} else {
None
}
}
PatKind::Wild => Some(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))
None
}
}))
}

12
compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file

@ -2514,12 +2514,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
let ty::Tuple(params) = tupled_params.kind() else { return };
// Find the first argument with a matching type, get its name
let Some((_, this_name)) =
params.iter().zip(tcx.hir_body_param_names(closure.body)).find(|(param_ty, name)| {
let Some(this_name) = params.iter().zip(tcx.hir_body_param_names(closure.body)).find_map(
|(param_ty, name)| {
// FIXME: also support deref for stuff like `Rc` arguments
param_ty.peel_refs() == local_ty && name != &Ident::empty()
})
else {
if param_ty.peel_refs() == local_ty { name } else { None }
},
) else {
return;
};
@ -3787,7 +3787,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
method_args,
*fn_span,
call_source.from_hir_call(),
Some(self.infcx.tcx.fn_arg_names(method_did)[0]),
self.infcx.tcx.fn_arg_names(method_did)[0],
)
{
err.note(format!("borrow occurs due to deref coercion to `{deref_target_ty}`"));

2
compiler/rustc_borrowck/src/diagnostics/mod.rs
View file

@ -1029,7 +1029,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
method_args,
*fn_span,
call_source.from_hir_call(),
Some(self.infcx.tcx.fn_arg_names(method_did)[0]),
self.infcx.tcx.fn_arg_names(method_did)[0],
);
return FnSelfUse {

13
compiler/rustc_hir/src/hir.rs
View file

@ -2949,7 +2949,7 @@ impl<'hir> TraitItem<'hir> {
#[derive(Debug, Clone, Copy, HashStable_Generic)]
pub enum TraitFn<'hir> {
/// No default body in the trait, just a signature.
Required(&'hir [Ident]),
Required(&'hir [Option<Ident>]),
/// Both signature and body are provided in the trait.
Provided(BodyId),
@ -3354,7 +3354,9 @@ pub struct BareFnTy<'hir> {
pub abi: ExternAbi,
pub generic_params: &'hir [GenericParam<'hir>],
pub decl: &'hir FnDecl<'hir>,
pub param_names: &'hir [Ident],
// `Option` because bare fn parameter names are optional. We also end up
// with `None` in some error cases, e.g. invalid parameter patterns.
pub param_names: &'hir [Option<Ident>],
}
#[derive(Debug, Clone, Copy, HashStable_Generic)]
@ -4335,7 +4337,12 @@ impl ForeignItem<'_> {
#[derive(Debug, Clone, Copy, HashStable_Generic)]
pub enum ForeignItemKind<'hir> {
/// A foreign function.
Fn(FnSig<'hir>, &'hir [Ident], &'hir Generics<'hir>),
///
/// All argument idents are actually always present (i.e. `Some`), but
/// `&[Option<Ident>]` is used because of code paths shared with `TraitFn`
/// and `BareFnTy`. The sharing is due to all of these cases not allowing
/// arbitrary patterns for parameters.
Fn(FnSig<'hir>, &'hir [Option<Ident>], &'hir Generics<'hir>),
/// A foreign static item (`static ext: u8`).
Static(&'hir Ty<'hir>, Mutability, Safety),
/// A foreign type.

8
compiler/rustc_hir/src/intravisit.rs
View file

@ -655,7 +655,9 @@ pub fn walk_foreign_item<'v, V: Visitor<'v>>(
ForeignItemKind::Fn(ref sig, param_names, ref generics) => {
try_visit!(visitor.visit_generics(generics));
try_visit!(visitor.visit_fn_decl(sig.decl));
walk_list!(visitor, visit_ident, param_names.iter().copied());
for ident in param_names.iter().copied() {
visit_opt!(visitor, visit_ident, ident);
}
}
ForeignItemKind::Static(ref typ, _, _) => {
try_visit!(visitor.visit_ty_unambig(typ));
@ -1169,7 +1171,9 @@ pub fn walk_trait_item<'v, V: Visitor<'v>>(
}
TraitItemKind::Fn(ref sig, TraitFn::Required(param_names)) => {
try_visit!(visitor.visit_fn_decl(sig.decl));
walk_list!(visitor, visit_ident, param_names.iter().copied());
for ident in param_names.iter().copied() {
visit_opt!(visitor, visit_ident, ident);
}
}
TraitItemKind::Fn(ref sig, TraitFn::Provided(body_id)) => {
try_visit!(visitor.visit_fn(

8
compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
View file

@ -1034,7 +1034,13 @@ fn report_trait_method_mismatch<'tcx>(
let span = tcx
.hir_body_param_names(body)
.zip(sig.decl.inputs.iter())
.map(|(param, ty)| param.span.to(ty.span))
.map(|(param_name, ty)| {
if let Some(param_name) = param_name {
param_name.span.to(ty.span)
} else {
ty.span
}
})
.next()
.unwrap_or(impl_err_span);

8
compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs
View file

@ -49,9 +49,11 @@ pub(crate) fn validate_cmse_abi<'tcx>(
Ok(Err(index)) => {
// fn(x: u32, u32, u32, u16, y: u16) -> u32,
// ^^^^^^
let span = bare_fn_ty.param_names[index]
.span
.to(bare_fn_ty.decl.inputs[index].span)
let span = if let Some(ident) = bare_fn_ty.param_names[index] {
ident.span.to(bare_fn_ty.decl.inputs[index].span)
} else {
bare_fn_ty.decl.inputs[index].span
}
.to(bare_fn_ty.decl.inputs.last().unwrap().span);
let plural = bare_fn_ty.param_names.len() - index != 1;
dcx.emit_err(errors::CmseInputsStackSpill { span, plural, abi });

9
compiler/rustc_hir_pretty/src/lib.rs
View file

@ -2,6 +2,7 @@
//! the definitions in this file have equivalents in `rustc_ast_pretty`.
// tidy-alphabetical-start
#![feature(let_chains)]
#![recursion_limit = "256"]
// tidy-alphabetical-end
@ -898,7 +899,7 @@ impl<'a> State<'a> {
ident: Ident,
m: &hir::FnSig<'_>,
generics: &hir::Generics<'_>,
arg_names: &[Ident],
arg_names: &[Option<Ident>],
body_id: Option<hir::BodyId>,
) {
self.print_fn(m.decl, m.header, Some(ident.name), generics, arg_names, body_id);
@ -2121,7 +2122,7 @@ impl<'a> State<'a> {
header: hir::FnHeader,
name: Option<Symbol>,
generics: &hir::Generics<'_>,
arg_names: &[Ident],
arg_names: &[Option<Ident>],
body_id: Option<hir::BodyId>,
) {
self.print_fn_header_info(header);
@ -2141,7 +2142,7 @@ impl<'a> State<'a> {
s.print_implicit_self(&decl.implicit_self);
} else {
if let Some(arg_name) = arg_names.get(i) {
if arg_name.name != kw::Empty {
if let Some(arg_name) = arg_name {
s.word(arg_name.to_string());
s.word(":");
s.space();
@ -2451,7 +2452,7 @@ impl<'a> State<'a> {
decl: &hir::FnDecl<'_>,
name: Option<Symbol>,
generic_params: &[hir::GenericParam<'_>],
arg_names: &[Ident],
arg_names: &[Option<Ident>],
) {
self.ibox(INDENT_UNIT);
self.print_formal_generic_params(generic_params);

17
compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
View file

@ -1135,7 +1135,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&& self.tcx.def_kind(fn_def_id).is_fn_like()
&& let self_implicit =
matches!(call_expr.kind, hir::ExprKind::MethodCall(..)) as usize
&& let Some(arg) =
&& let Some(Some(arg)) =
self.tcx.fn_arg_names(fn_def_id).get(expected_idx.as_usize() + self_implicit)
&& arg.name != kw::SelfLower
{
@ -2678,7 +2678,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(|&ident| FnParam::Name(ident))).collect(),
generics,
))
}
@ -2709,14 +2709,20 @@ impl<'tcx> Visitor<'tcx> for FindClosureArg<'tcx> {
#[derive(Clone, Copy)]
enum FnParam<'hir> {
Param(&'hir hir::Param<'hir>),
Name(Ident),
Name(Option<Ident>),
}
impl FnParam<'_> {
fn span(&self) -> Span {
match self {
Self::Param(param) => param.span,
Self::Name(ident) => ident.span,
Self::Name(ident) => {
if let Some(ident) = ident {
ident.span
} else {
DUMMY_SP
}
}
}
}
@ -2733,7 +2739,8 @@ impl FnParam<'_> {
Some(ident.name)
}
FnParam::Name(ident)
if ident.name != kw::Empty && ident.name != kw::Underscore =>
if let Some(ident) = ident
&& ident.name != kw::Underscore =>
{
Some(ident.name)
}

2
compiler/rustc_hir_typeck/src/method/suggest.rs
View file

@ -3766,7 +3766,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
{
let self_first_arg = match method {
hir::TraitFn::Required([ident, ..]) => {
ident.name == kw::SelfLower
matches!(ident, Some(Ident { name: kw::SelfLower, .. }))
}
hir::TraitFn::Provided(body_id) => {
self.tcx.hir_body(*body_id).params.first().is_some_and(

2
compiler/rustc_lint/src/nonstandard_style.rs
View file

@ -424,10 +424,12 @@ impl<'tcx> LateLintPass<'tcx> for NonSnakeCase {
if let hir::TraitItemKind::Fn(_, hir::TraitFn::Required(pnames)) = item.kind {
self.check_snake_case(cx, "trait method", &item.ident);
for param_name in pnames {
if let Some(param_name) = param_name {
self.check_snake_case(cx, "variable", param_name);
}
}
}
}
fn check_pat(&mut self, cx: &LateContext<'_>, p: &hir::Pat<'_>) {
if let PatKind::Binding(_, hid, ident, _) = p.kind {

2
compiler/rustc_metadata/src/rmeta/decoder.rs
View file

@ -1318,7 +1318,7 @@ impl<'a> CrateMetadataRef<'a> {
.expect("argument names not encoded for a function")
.decode((self, sess))
.nth(0)
.is_some_and(|ident| ident.name == kw::SelfLower)
.is_some_and(|ident| matches!(ident, Some(Ident { name: kw::SelfLower, .. })))
}
fn get_associated_item_or_field_def_ids(self, id: DefIndex) -> impl Iterator<Item = DefId> {

2
compiler/rustc_metadata/src/rmeta/mod.rs
View file

@ -443,7 +443,7 @@ define_tables! {
rendered_const: Table<DefIndex, LazyValue<String>>,
rendered_precise_capturing_args: Table<DefIndex, LazyArray<PreciseCapturingArgKind<Symbol, Symbol>>>,
asyncness: Table<DefIndex, ty::Asyncness>,
fn_arg_names: Table<DefIndex, LazyArray<Ident>>,
fn_arg_names: Table<DefIndex, LazyArray<Option<Ident>>>,
coroutine_kind: Table<DefIndex, hir::CoroutineKind>,
coroutine_for_closure: Table<DefIndex, RawDefId>,
coroutine_by_move_body_def_id: Table<DefIndex, RawDefId>,

8
compiler/rustc_middle/src/hir/map.rs
View file

@ -280,11 +280,11 @@ impl<'tcx> TyCtxt<'tcx> {
})
}
pub fn hir_body_param_names(self, id: BodyId) -> impl Iterator<Item = Ident> {
pub fn hir_body_param_names(self, id: BodyId) -> impl Iterator<Item = Option<Ident>> {
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(),
PatKind::Binding(_, _, ident, _) => Some(ident),
PatKind::Wild => Some(Ident::new(kw::Underscore, param.pat.span)),
_ => None,
})
}

2
compiler/rustc_middle/src/query/mod.rs
View file

@ -1410,7 +1410,7 @@ rustc_queries! {
desc { |tcx| "computing target features for inline asm of `{}`", tcx.def_path_str(def_id) }
}
query fn_arg_names(def_id: DefId) -> &'tcx [rustc_span::Ident] {
query fn_arg_names(def_id: DefId) -> &'tcx [Option<rustc_span::Ident>] {
desc { |tcx| "looking up function parameter names for `{}`", tcx.def_path_str(def_id) }
separate_provide_extern
}

11
compiler/rustc_resolve/src/late/diagnostics.rs
View file

@ -2217,12 +2217,11 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
.delegation_fn_sigs
.get(&def_id)
.is_some_and(|sig| sig.has_self),
None => self
.r
.tcx
.fn_arg_names(def_id)
.first()
.is_some_and(|ident| ident.name == kw::SelfLower),
None => {
self.r.tcx.fn_arg_names(def_id).first().is_some_and(|&ident| {
matches!(ident, Some(Ident { name: kw::SelfLower, .. }))
})
}
};
if has_self {
return Some(AssocSuggestion::MethodWithSelf { called });

9
compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
View file

@ -1992,13 +1992,12 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
.iter()
.enumerate()
.map(|(i, ident)| {
if ident.name.is_empty()
|| ident.name == kw::Underscore
|| ident.name == kw::SelfLower
if let Some(ident) = ident
&& !matches!(ident, Ident { name: kw::Underscore | kw::SelfLower, .. })
{
format!("arg{i}")
} else {
format!("{ident}")
} else {
format!("arg{i}")
}
})
.collect();

24
src/librustdoc/clean/mod.rs
View file

@ -1088,7 +1088,7 @@ fn clean_fn_decl_legacy_const_generics(func: &mut Function, attrs: &[hir::Attrib
enum FunctionArgs<'tcx> {
Body(hir::BodyId),
Names(&'tcx [Ident]),
Names(&'tcx [Option<Ident>]),
}
fn clean_function<'tcx>(
@ -1117,13 +1117,15 @@ fn clean_function<'tcx>(
fn clean_args_from_types_and_names<'tcx>(
cx: &mut DocContext<'tcx>,
types: &[hir::Ty<'tcx>],
names: &[Ident],
names: &[Option<Ident>],
) -> Arguments {
fn nonempty_name(ident: &Ident) -> Option<Symbol> {
if ident.name == kw::Underscore || ident.name == kw::Empty {
None
} else {
fn nonempty_name(ident: &Option<Ident>) -> Option<Symbol> {
if let Some(ident) = ident
&& ident.name != kw::Underscore
{
Some(ident.name)
} else {
None
}
}
@ -1216,11 +1218,11 @@ fn clean_poly_fn_sig<'tcx>(
.iter()
.map(|t| Argument {
type_: clean_middle_ty(t.map_bound(|t| *t), cx, None, None),
name: names
.next()
.map(|i| i.name)
.filter(|i| !i.is_empty())
.unwrap_or(kw::Underscore),
name: if let Some(Some(ident)) = names.next() {
ident.name
} else {
kw::Underscore
},
is_const: false,
})
.collect(),

43
src/tools/clippy/clippy_lints/src/functions/renamed_function_params.rs
View file

@ -5,7 +5,7 @@ use rustc_hir::hir_id::OwnerId;
use rustc_hir::{Impl, ImplItem, ImplItemKind, ImplItemRef, ItemKind, Node, TraitRef};
use rustc_lint::LateContext;
use rustc_span::Span;
use rustc_span::symbol::{Ident, Symbol, kw};
use rustc_span::symbol::{Ident, kw};
use super::RENAMED_FUNCTION_PARAMS;
@ -51,22 +51,33 @@ struct RenamedFnArgs(Vec<(Span, String)>);
impl RenamedFnArgs {
/// Comparing between an iterator of default names and one with current names,
/// then collect the ones that got renamed.
fn new<I, T>(default_names: &mut I, current_names: &mut T) -> Self
fn new<I1, I2>(default_idents: &mut I1, current_idents: &mut I2) -> Self
where
I: Iterator<Item = Ident>,
T: Iterator<Item = Ident>,
I1: Iterator<Item = Option<Ident>>,
I2: Iterator<Item = Option<Ident>>,
{
let mut renamed: Vec<(Span, String)> = vec![];
debug_assert!(default_names.size_hint() == current_names.size_hint());
while let (Some(def_name), Some(cur_name)) = (default_names.next(), current_names.next()) {
let current_name = cur_name.name;
let default_name = def_name.name;
if is_unused_or_empty_symbol(current_name) || is_unused_or_empty_symbol(default_name) {
continue;
debug_assert!(default_idents.size_hint() == current_idents.size_hint());
while let (Some(default_ident), Some(current_ident)) =
(default_idents.next(), current_idents.next())
{
let has_name_to_check = |ident: Option<Ident>| {
if let Some(ident) = ident
&& ident.name != kw::Underscore
&& !ident.name.as_str().starts_with('_')
{
Some(ident)
} else {
None
}
if current_name != default_name {
renamed.push((cur_name.span, default_name.to_string()));
};
if let Some(default_ident) = has_name_to_check(default_ident)
&& let Some(current_ident) = has_name_to_check(current_ident)
&& default_ident.name != current_ident.name
{
renamed.push((current_ident.span, default_ident.to_string()));
}
}
@ -83,14 +94,6 @@ impl RenamedFnArgs {
}
}
fn is_unused_or_empty_symbol(symbol: Symbol) -> bool {
// FIXME: `body_param_names` currently returning empty symbols for `wild` as well,
// so we need to check if the symbol is empty first.
// Therefore the check of whether it's equal to [`kw::Underscore`] has no use for now,
// but it would be nice to keep it here just to be future-proof.
symbol.is_empty() || symbol == kw::Underscore || symbol.as_str().starts_with('_')
}
/// Get the [`trait_item_def_id`](ImplItemRef::trait_item_def_id) of a relevant impl item.
fn trait_item_def_id_of_impl(items: &[ImplItemRef], target: OwnerId) -> Option<DefId> {
items.iter().find_map(|item| {

10
src/tools/clippy/clippy_lints/src/lifetimes.rs
View file

@ -189,7 +189,7 @@ fn check_fn_inner<'tcx>(
cx: &LateContext<'tcx>,
sig: &'tcx FnSig<'_>,
body: Option<BodyId>,
trait_sig: Option<&[Ident]>,
trait_sig: Option<&[Option<Ident>]>,
generics: &'tcx Generics<'_>,
span: Span,
report_extra_lifetimes: bool,
@ -264,7 +264,7 @@ fn could_use_elision<'tcx>(
cx: &LateContext<'tcx>,
func: &'tcx FnDecl<'_>,
body: Option<BodyId>,
trait_sig: Option<&[Ident]>,
trait_sig: Option<&[Option<Ident>]>,
named_generics: &'tcx [GenericParam<'_>],
msrv: Msrv,
) -> Option<(Vec<LocalDefId>, Vec<Lifetime>)> {
@ -310,7 +310,7 @@ fn could_use_elision<'tcx>(
let body = cx.tcx.hir_body(body_id);
let first_ident = body.params.first().and_then(|param| param.pat.simple_ident());
if non_elidable_self_type(cx, func, first_ident, msrv) {
if non_elidable_self_type(cx, func, Some(first_ident), msrv) {
return None;
}
@ -384,8 +384,8 @@ fn allowed_lts_from(named_generics: &[GenericParam<'_>]) -> FxIndexSet<LocalDefI
}
// elision doesn't work for explicit self types before Rust 1.81, see rust-lang/rust#69064
fn non_elidable_self_type<'tcx>(cx: &LateContext<'tcx>, func: &FnDecl<'tcx>, ident: Option<Ident>, msrv: Msrv) -> bool {
if let Some(ident) = ident
fn non_elidable_self_type<'tcx>(cx: &LateContext<'tcx>, func: &FnDecl<'tcx>, ident: Option<Option<Ident>>, msrv: Msrv) -> bool {
if let Some(Some(ident)) = ident
&& ident.name == kw::SelfLower
&& !func.implicit_self.has_implicit_self()
&& let Some(self_ty) = func.inputs.first()