Skip to content
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

Use Option<Ident> for lowered param names. #138685

Merged
Merged
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
14 changes: 10 additions & 4 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 6 additions & 6 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 10 additions & 3 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 6 additions & 2 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 7 additions & 1 deletion compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
@@ -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);

10 changes: 6 additions & 4 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs
Original file line number Diff line number Diff line change
@@ -49,10 +49,12 @@ 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)
.to(bare_fn_ty.decl.inputs.last().unwrap().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 changes: 5 additions & 4 deletions compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 12 additions & 5 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
@@ -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(
4 changes: 3 additions & 1 deletion compiler/rustc_lint/src/nonstandard_style.rs
Original file line number Diff line number Diff line change
@@ -424,7 +424,9 @@ 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 {
self.check_snake_case(cx, "variable", param_name);
if let Some(param_name) = param_name {
self.check_snake_case(cx, "variable", param_name);
}
}
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 4 additions & 4 deletions compiler/rustc_middle/src/hir/map.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 5 additions & 6 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -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 });
Original file line number Diff line number Diff line change
@@ -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 changes: 13 additions & 11 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
@@ -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(),
Original file line number Diff line number Diff line change
@@ -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;
}
if current_name != default_name {
renamed.push((cur_name.span, default_name.to_string()));
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 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 changes: 5 additions & 5 deletions src/tools/clippy/clippy_lints/src/lifetimes.rs
Original file line number Diff line number Diff line change
@@ -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()