Skip to content

rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions) #139913

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.

Sign up for GitHub

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 2 commits into from
Apr 18, 2025
+186 −167
Merged

rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions) #139913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6214674
rustdoc/clean: Change terminology of items pertaining to (formal) fn …
fmease Apr 15, 2025
82ff0a0
rustdoc: Properly clean fn params in all contexts
fmease Apr 16, 2025
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
188 changes: 82 additions & 106 deletions src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,7 @@ fn clean_fn_or_proc_macro<'tcx>(
match macro_kind {
Some(kind) => clean_proc_macro(item, name, kind, cx),
None => {
let mut func = clean_function(cx, sig, generics, FunctionArgs::Body(body_id));
let mut func = clean_function(cx, sig, generics, ParamsSrc::Body(body_id));
clean_fn_decl_legacy_const_generics(&mut func, attrs);
FunctionItem(func)
}
Expand All @@ -1071,16 +1071,11 @@ fn clean_fn_decl_legacy_const_generics(func: &mut Function, attrs: &[hir::Attrib
for (pos, literal) in meta_item_list.iter().filter_map(|meta| meta.lit()).enumerate() {
match literal.kind {
ast::LitKind::Int(a, _) => {
let param = func.generics.params.remove(0);
if let GenericParamDef {
name,
kind: GenericParamDefKind::Const { ty, .. },
..
} = param
{
func.decl.inputs.values.insert(
let GenericParamDef { name, kind, .. } = func.generics.params.remove(0);
if let GenericParamDefKind::Const { ty, .. } = kind {
func.decl.inputs.insert(
a.get() as _,
Argument { name: Some(name), type_: *ty, is_const: true },
Parameter { name: Some(name), type_: *ty, is_const: true },
);
} else {
panic!("unexpected non const in position {pos}");
Expand All @@ -1092,7 +1087,7 @@ fn clean_fn_decl_legacy_const_generics(func: &mut Function, attrs: &[hir::Attrib
}
}

enum FunctionArgs<'tcx> {
enum ParamsSrc<'tcx> {
Body(hir::BodyId),
Idents(&'tcx [Option<Ident>]),
}
Expand All @@ -1101,86 +1096,62 @@ fn clean_function<'tcx>(
cx: &mut DocContext<'tcx>,
sig: &hir::FnSig<'tcx>,
generics: &hir::Generics<'tcx>,
args: FunctionArgs<'tcx>,
params: ParamsSrc<'tcx>,
) -> Box<Function> {
let (generics, decl) = enter_impl_trait(cx, |cx| {
// NOTE: generics must be cleaned before args
// NOTE: Generics must be cleaned before params.
let generics = clean_generics(generics, cx);
let args = match args {
FunctionArgs::Body(body_id) => {
clean_args_from_types_and_body_id(cx, sig.decl.inputs, body_id)
}
FunctionArgs::Idents(idents) => {
clean_args_from_types_and_names(cx, sig.decl.inputs, idents)
}
let params = match params {
ParamsSrc::Body(body_id) => clean_params_via_body(cx, sig.decl.inputs, body_id),
// Let's not perpetuate anon params from Rust 2015; use `_` for them.
ParamsSrc::Idents(idents) => clean_params(cx, sig.decl.inputs, idents, |ident| {
Some(ident.map_or(kw::Underscore, |ident| ident.name))
}),
};
let decl = clean_fn_decl_with_args(cx, sig.decl, Some(&sig.header), args);
let decl = clean_fn_decl_with_params(cx, sig.decl, Some(&sig.header), params);
(generics, decl)
});
Box::new(Function { decl, generics })
}

fn clean_args_from_types_and_names<'tcx>(
fn clean_params<'tcx>(
cx: &mut DocContext<'tcx>,
types: &[hir::Ty<'tcx>],
idents: &[Option<Ident>],
) -> Arguments {
fn nonempty_name(ident: &Option<Ident>) -> Option<Symbol> {
if let Some(ident) = ident
&& ident.name != kw::Underscore
{
Some(ident.name)
} else {
None
}
}

// If at least one argument has a name, use `_` as the name of unnamed
// arguments. Otherwise omit argument names.
let default_name = if idents.iter().any(|ident| nonempty_name(ident).is_some()) {
Some(kw::Underscore)
} else {
None
};

Arguments {
values: types
.iter()
.enumerate()
.map(|(i, ty)| Argument {
type_: clean_ty(ty, cx),
name: idents.get(i).and_then(nonempty_name).or(default_name),
is_const: false,
})
.collect(),
}
postprocess: impl Fn(Option<Ident>) -> Option<Symbol>,
) -> Vec<Parameter> {
types
.iter()
.enumerate()
.map(|(i, ty)| Parameter {
name: postprocess(idents[i]),
type_: clean_ty(ty, cx),
is_const: false,
})
.collect()
}

fn clean_args_from_types_and_body_id<'tcx>(
fn clean_params_via_body<'tcx>(
cx: &mut DocContext<'tcx>,
types: &[hir::Ty<'tcx>],
body_id: hir::BodyId,
) -> Arguments {
let body = cx.tcx.hir_body(body_id);

Arguments {
values: types
.iter()
.zip(body.params)
.map(|(ty, param)| Argument {
name: Some(name_from_pat(param.pat)),
type_: clean_ty(ty, cx),
is_const: false,
})
.collect(),
}
) -> Vec<Parameter> {
types
.iter()
.zip(cx.tcx.hir_body(body_id).params)
.map(|(ty, param)| Parameter {
name: Some(name_from_pat(param.pat)),
type_: clean_ty(ty, cx),
is_const: false,
})
.collect()
}

fn clean_fn_decl_with_args<'tcx>(
fn clean_fn_decl_with_params<'tcx>(
cx: &mut DocContext<'tcx>,
decl: &hir::FnDecl<'tcx>,
header: Option<&hir::FnHeader>,
args: Arguments,
params: Vec<Parameter>,
) -> FnDecl {
let mut output = match decl.output {
hir::FnRetTy::Return(typ) => clean_ty(typ, cx),
Expand All @@ -1191,18 +1162,14 @@ fn clean_fn_decl_with_args<'tcx>(
{
output = output.sugared_async_return_type();
}
FnDecl { inputs: args, output, c_variadic: decl.c_variadic }
FnDecl { inputs: params, output, c_variadic: decl.c_variadic }
}

fn clean_poly_fn_sig<'tcx>(
cx: &mut DocContext<'tcx>,
did: Option<DefId>,
sig: ty::PolyFnSig<'tcx>,
) -> FnDecl {
let mut names = did.map_or(&[] as &[_], |did| cx.tcx.fn_arg_idents(did)).iter();

// We assume all empty tuples are default return type. This theoretically can discard `-> ()`,
// but shouldn't change any code meaning.
let mut output = clean_middle_ty(sig.output(), cx, None, None);

// If the return type isn't an `impl Trait`, we can safely assume that this
Expand All @@ -1215,25 +1182,25 @@ fn clean_poly_fn_sig<'tcx>(
output = output.sugared_async_return_type();
}

FnDecl {
output,
c_variadic: sig.skip_binder().c_variadic,
inputs: Arguments {
values: sig
.inputs()
.iter()
.map(|t| Argument {
type_: clean_middle_ty(t.map_bound(|t| *t), cx, None, None),
name: Some(if let Some(Some(ident)) = names.next() {
ident.name
} else {
kw::Underscore
}),
is_const: false,
})
.collect(),
},
}
let mut idents = did.map(|did| cx.tcx.fn_arg_idents(did)).unwrap_or_default().iter().copied();

// If this comes from a fn item, let's not perpetuate anon params from Rust 2015; use `_` for them.
// If this comes from a fn ptr ty, we just keep params unnamed since it's more conventional stylistically.
// Since the param name is not part of the semantic type, these params never bear a name unlike
// in the HIR case, thus we can't peform any fancy fallback logic unlike `clean_bare_fn_ty`.
let fallback = did.map(|_| kw::Underscore);

let params = sig
.inputs()
.iter()
.map(|ty| Parameter {
name: idents.next().flatten().map(|ident| ident.name).or(fallback),
type_: clean_middle_ty(ty.map_bound(|ty| *ty), cx, None, None),
is_const: false,
})
.collect();

FnDecl { inputs: params, output, c_variadic: sig.skip_binder().c_variadic }
}

fn clean_trait_ref<'tcx>(trait_ref: &hir::TraitRef<'tcx>, cx: &mut DocContext<'tcx>) -> Path {
Expand Down Expand Up @@ -1273,11 +1240,11 @@ fn clean_trait_item<'tcx>(trait_item: &hir::TraitItem<'tcx>, cx: &mut DocContext
RequiredAssocConstItem(generics, Box::new(clean_ty(ty, cx)))
}
hir::TraitItemKind::Fn(ref sig, hir::TraitFn::Provided(body)) => {
let m = clean_function(cx, sig, trait_item.generics, FunctionArgs::Body(body));
let m = clean_function(cx, sig, trait_item.generics, ParamsSrc::Body(body));
MethodItem(m, None)
}
hir::TraitItemKind::Fn(ref sig, hir::TraitFn::Required(idents)) => {
let m = clean_function(cx, sig, trait_item.generics, FunctionArgs::Idents(idents));
let m = clean_function(cx, sig, trait_item.generics, ParamsSrc::Idents(idents));
RequiredMethodItem(m)
}
hir::TraitItemKind::Type(bounds, Some(default)) => {
Expand Down Expand Up @@ -1318,7 +1285,7 @@ pub(crate) fn clean_impl_item<'tcx>(
type_: clean_ty(ty, cx),
})),
hir::ImplItemKind::Fn(ref sig, body) => {
let m = clean_function(cx, sig, impl_.generics, FunctionArgs::Body(body));
let m = clean_function(cx, sig, impl_.generics, ParamsSrc::Body(body));
let defaultness = cx.tcx.defaultness(impl_.owner_id);
MethodItem(m, Some(defaultness))
}
Expand Down Expand Up @@ -1390,14 +1357,14 @@ pub(crate) fn clean_middle_assoc_item(assoc_item: &ty::AssocItem, cx: &mut DocCo
}
ty::AssocItemContainer::Trait => tcx.types.self_param,
};
let self_arg_ty =
let self_param_ty =
tcx.fn_sig(assoc_item.def_id).instantiate_identity().input(0).skip_binder();
if self_arg_ty == self_ty {
item.decl.inputs.values[0].type_ = SelfTy;
} else if let ty::Ref(_, ty, _) = *self_arg_ty.kind()
if self_param_ty == self_ty {
item.decl.inputs[0].type_ = SelfTy;
} else if let ty::Ref(_, ty, _) = *self_param_ty.kind()
&& ty == self_ty
{
match item.decl.inputs.values[0].type_ {
match item.decl.inputs[0].type_ {
BorrowedRef { ref mut type_, .. } => **type_ = SelfTy,
_ => unreachable!(),
}
Expand Down Expand Up @@ -2611,15 +2578,25 @@ fn clean_bare_fn_ty<'tcx>(
cx: &mut DocContext<'tcx>,
) -> BareFunctionDecl {
let (generic_params, decl) = enter_impl_trait(cx, |cx| {
// NOTE: generics must be cleaned before args
// NOTE: Generics must be cleaned before params.
let generic_params = bare_fn
.generic_params
.iter()
.filter(|p| !is_elided_lifetime(p))
.map(|x| clean_generic_param(cx, None, x))
.collect();
let args = clean_args_from_types_and_names(cx, bare_fn.decl.inputs, bare_fn.param_idents);
let decl = clean_fn_decl_with_args(cx, bare_fn.decl, None, args);
// Since it's more conventional stylistically, elide the name of all params called `_`
// unless there's at least one interestingly named param in which case don't elide any
// name since mixing named and unnamed params is less legible.
let filter = |ident: Option<Ident>| {
ident.map(|ident| ident.name).filter(|&ident| ident != kw::Underscore)
};
let fallback =
bare_fn.param_idents.iter().copied().find_map(filter).map(|_| kw::Underscore);
let params = clean_params(cx, bare_fn.decl.inputs, bare_fn.param_idents, |ident| {
filter(ident).or(fallback)
});
let decl = clean_fn_decl_with_params(cx, bare_fn.decl, None, params);
(generic_params, decl)
});
BareFunctionDecl { safety: bare_fn.safety, abi: bare_fn.abi, decl, generic_params }
Expand All @@ -2629,7 +2606,6 @@ fn clean_unsafe_binder_ty<'tcx>(
unsafe_binder_ty: &hir::UnsafeBinderTy<'tcx>,
cx: &mut DocContext<'tcx>,
) -> UnsafeBinderTy {
// NOTE: generics must be cleaned before args
let generic_params = unsafe_binder_ty
.generic_params
.iter()
Expand Down Expand Up @@ -3155,7 +3131,7 @@ fn clean_maybe_renamed_foreign_item<'tcx>(
cx.with_param_env(def_id, |cx| {
let kind = match item.kind {
hir::ForeignItemKind::Fn(sig, idents, generics) => ForeignFunctionItem(
clean_function(cx, &sig, generics, FunctionArgs::Idents(idents)),
clean_function(cx, &sig, generics, ParamsSrc::Idents(idents)),
sig.header.safety(),
),
hir::ForeignItemKind::Static(ty, mutability, safety) => ForeignStaticItem(
Expand Down
16 changes: 6 additions & 10 deletions src/librustdoc/clean/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1407,32 +1407,28 @@ pub(crate) struct Function {

#[derive(Clone, PartialEq, Eq, Debug, Hash)]
pub(crate) struct FnDecl {
pub(crate) inputs: Arguments,
pub(crate) inputs: Vec<Parameter>,
pub(crate) output: Type,
pub(crate) c_variadic: bool,
}

impl FnDecl {
pub(crate) fn receiver_type(&self) -> Option<&Type> {
self.inputs.values.first().and_then(|v| v.to_receiver())
self.inputs.first().and_then(|v| v.to_receiver())
}
}

/// A function parameter.
#[derive(Clone, PartialEq, Eq, Debug, Hash)]
pub(crate) struct Arguments {
pub(crate) values: Vec<Argument>,
}

#[derive(Clone, PartialEq, Eq, Debug, Hash)]
pub(crate) struct Argument {
pub(crate) type_: Type,
pub(crate) struct Parameter {
pub(crate) name: Option<Symbol>,
pub(crate) type_: Type,
/// This field is used to represent "const" arguments from the `rustc_legacy_const_generics`
/// feature. More information in <https://github.com/rust-lang/rust/issues/83167>.
pub(crate) is_const: bool,
}

impl Argument {
impl Parameter {
pub(crate) fn to_receiver(&self) -> Option<&Type> {
if self.name == Some(kw::SelfLower) { Some(&self.type_) } else { None }
}
Expand Down
9 changes: 4 additions & 5 deletions src/librustdoc/clean/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,13 +303,12 @@ pub(crate) fn name_from_pat(p: &hir::Pat<'_>) -> Symbol {
debug!("trying to get a name from pattern: {p:?}");

Symbol::intern(&match &p.kind {
// FIXME(never_patterns): does this make sense?
PatKind::Missing => unreachable!(),
PatKind::Wild
| PatKind::Err(_)
PatKind::Err(_)
| PatKind::Missing // Let's not perpetuate anon params from Rust 2015; use `_` for them.
| PatKind::Never
| PatKind::Range(..)
| PatKind::Struct(..)
| PatKind::Range(..) => {
| PatKind::Wild => {
return kw::Underscore;
}
PatKind::Binding(_, _, ident, _) => return ident.name,
Expand Down
Loading
Loading