Skip to content

Hide host effect params from docs #116670

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
bors merged 6 commits into from
Oct 13, 2023
Merged

Hide host effect params from docs #116670

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
20363f4
Add regression tests
oli-obk Oct 12, 2023
c4e61fa
Hide host effect params from docs
oli-obk Oct 12, 2023
8f2af7e
Test cross crate
oli-obk Oct 12, 2023
cfb6afa
Add regression test for generic args showing `host` param
oli-obk Oct 12, 2023
6724f99
hide `host` param from generic parameter list of `~const` bounds
oli-obk Oct 12, 2023
16f8396
Add some FIXMEs for remaining issues that we need to fix before using…
oli-obk Oct 13, 2023
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
4 changes: 3 additions & 1 deletion src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ fn clean_generic_param_def<'tcx>(
},
)
}
ty::GenericParamDefKind::Const { has_default, .. } => (
ty::GenericParamDefKind::Const { has_default, is_host_effect } => (
def.name,
GenericParamDefKind::Const {
ty: Box::new(clean_middle_ty(
Expand All @@ -541,6 +541,7 @@ fn clean_generic_param_def<'tcx>(
)),
false => None,
},
is_host_effect,
},
),
};
Expand Down Expand Up @@ -597,6 +598,7 @@ fn clean_generic_param<'tcx>(
ty: Box::new(clean_ty(ty, cx)),
default: default
.map(|ct| Box::new(ty::Const::from_anon_const(cx.tcx, ct.def_id).to_string())),
is_host_effect: cx.tcx.has_attr(param.def_id, sym::rustc_host),
Copy link
Copy Markdown
Contributor

@compiler-errors compiler-errors Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we re-encoding this here? Why not just call has_attr when needed?

Copy link
Copy Markdown
Contributor Author

@oli-obk oli-obk Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this works if it comes from the HIR, but that's not always true, there is also a code path where the is_host_effect comes from a ty::GenericParamDef due to coming from a different crate. I don't have tests for this, but that's the reason why we encode this field in ty::GenericParamDef instead of just loading the information lazily.

Copy link
Copy Markdown
Contributor

@compiler-errors compiler-errors Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can call tcx.has_attr on a foreign param, though, and we should be encoding attrs for foreign params too.

Copy link
Copy Markdown
Contributor Author

@oli-obk oli-obk Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we were filtering out most of them.

@fee1-dead do you remember why we put the is_host_effect field on GenericParamDef? did I ask for it just to call has_attr less?

Copy link
Copy Markdown
Member

@fmease fmease Oct 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#[rustc_host] used to be encoded in an earlier version of #115727 (84a4907) but since #115727 (comment) it no longer is.

Copy link
Copy Markdown
Member

@fee1-dead fee1-dead Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can call tcx.has_attr on a foreign param, though, and we should be encoding attrs for foreign params too.

We currently don't encode attributes on const generics params, and IIRC I added the field for that purpose

},
),
};
Expand Down
7 changes: 4 additions & 3 deletions src/librustdoc/clean/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1306,7 +1306,7 @@ impl WherePredicate {
pub(crate) enum GenericParamDefKind {
Lifetime { outlives: Vec<Lifetime> },
Type { did: DefId, bounds: Vec<GenericBound>, default: Option<Box<Type>>, synthetic: bool },
Const { ty: Box<Type>, default: Option<Box<String>> },
Const { ty: Box<Type>, default: Option<Box<String>>, is_host_effect: bool },
}

impl GenericParamDefKind {
Expand All @@ -1326,9 +1326,10 @@ impl GenericParamDef {
Self { name, kind: GenericParamDefKind::Lifetime { outlives: Vec::new() } }
}

pub(crate) fn is_synthetic_type_param(&self) -> bool {
pub(crate) fn is_synthetic_param(&self) -> bool {
match self.kind {
GenericParamDefKind::Lifetime { .. } | GenericParamDefKind::Const { .. } => false,
GenericParamDefKind::Lifetime { .. } => false,
GenericParamDefKind::Const { is_host_effect, .. } => is_host_effect,
GenericParamDefKind::Type { synthetic, .. } => synthetic,
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/librustdoc/html/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,7 @@ impl clean::Generics {
cx: &'a Context<'tcx>,
) -> impl fmt::Display + 'a + Captures<'tcx> {
display_fn(move |f| {
let mut real_params =
self.params.iter().filter(|p| !p.is_synthetic_type_param()).peekable();
let mut real_params = self.params.iter().filter(|p| !p.is_synthetic_param()).peekable();
if real_params.peek().is_none() {
return Ok(());
}
Expand Down
16 changes: 9 additions & 7 deletions src/librustdoc/json/conversions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ impl FromWithTcx<clean::GenericParamDefKind> for GenericParamDefKind {
default: default.map(|x| (*x).into_tcx(tcx)),
synthetic,
},
Const { ty, default } => GenericParamDefKind::Const {
Const { ty, default, is_host_effect: _ } => GenericParamDefKind::Const {
type_: (*ty).into_tcx(tcx),
default: default.map(|x| *x),
},
Expand Down Expand Up @@ -491,12 +491,14 @@ impl FromWithTcx<clean::WherePredicate> for WherePredicate {
default: default.map(|ty| (*ty).into_tcx(tcx)),
synthetic,
},
clean::GenericParamDefKind::Const { ty, default } => {
GenericParamDefKind::Const {
type_: (*ty).into_tcx(tcx),
default: default.map(|d| *d),
}
}
clean::GenericParamDefKind::Const {
ty,
default,
is_host_effect: _,
} => GenericParamDefKind::Const {
type_: (*ty).into_tcx(tcx),
default: default.map(|d| *d),
},
};
GenericParamDef { name, kind }
})
Expand Down
19 changes: 19 additions & 0 deletions tests/rustdoc/const-fn-effects.rs
View file
Open in desktop
Comment thread
oli-obk marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#![crate_name = "foo"]
#![feature(effects)]

// @has foo/fn.bar.html
// @has - '//pre[@class="rust item-decl"]' 'pub const fn bar() -> '
/// foo
pub const fn bar() -> usize {
2
}

// @has foo/struct.Foo.html
// @has - '//*[@class="method"]' 'const fn new()'
pub struct Foo(usize);

impl Foo {
pub const fn new() -> Foo {
Foo(0)
}
}