rustdoc: properly elide cross-crate host effect args #117531
Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
| @@ -2534,7 +2534,6 @@ fn clean_generic_args<'tcx>( | |||
| } | |||
| hir::GenericArg::Lifetime(_) => GenericArg::Lifetime(Lifetime::elided()), | |||
| hir::GenericArg::Type(ty) => GenericArg::Type(clean_ty(ty, cx)), | |||
| // FIXME(effects): This will still emit `<true>` for non-const impls of const traits | |||
There was a problem hiding this comment.
In #116670 (https://github.com/rust-lang/rust/pull/116670/files#r1357977394) I blamed this code path for /*host*/ true getting leaked in impls which isn't correct 😅. Now I fully understand the relevant parts of the effects lowering code and can confirm that this piece of code is correct. #112463 (sic!) fixed the specific impl issue.
Checking for the presence of #[rustc_host] on the AnonConst is correct and works not just for /*host*/ host but also for the hypothetical /*host*/ true (unreachable atm since true args never get synthesized in the first place) /*host*/ false (always const) (unreachable atm, #117530 would change that).
There was a problem hiding this comment.
I'm very tempted to ask you to add this as a code comment. So I'll succumb to the temptation: please add this as a code comment. :D
| // FIXME(effects): this causes `host = true` and `host = false` generics to also be emitted. | ||
| if let ty::ConstKind::Param(p) = ct.kind() | ||
| && p.name == sym::host | ||
| if let ty::GenericParamDefKind::Const { is_host_effect: true, .. } = params[index].kind |
There was a problem hiding this comment.
I'd prefer to have a test, even if pointless. If everything is covered, regressions will be easier to track down.
fmease
force-pushed
the
rustdoc-effects-properly-elide-x-crate-host-args
branch
from
1f74e13
to
ee0cdfd
Compare
fmease
force-pushed
the
rustdoc-effects-properly-elide-x-crate-host-args
branch
2 times, most recently
from
37c8feb
to
217153b
Compare
fmease
force-pushed
the
rustdoc-effects-properly-elide-x-crate-host-args
branch
from
217153b
to
1dcdf83
Compare
| // edition: 2021 | ||
| #![crate_name = "user"] | ||
|
|
||
| // Don't render the host param on `load` and the host arg `host` passed to `Resource`. |
There was a problem hiding this comment.
This one was already fixed by oli's PR but there was no cross-crate test for it yet, only a local crate one.
| // R: Resource" | ||
| pub use const_effect_param::load; | ||
|
|
||
| // Don't render the host arg `true` passed to `Resource`. |
There was a problem hiding this comment.
This one was fixed by my “elide cross-crate default generic args” PR which ofc predates oli's PR, hence no test until now. Feature effects synthesizes the param #[rustc_host_param] const host: bool = true for const fn etc, therefore since my PR was merged true gets elided.
This PR introduces a more principled approach and not only elides host and true but arbitrary const values like false. I can't add a test for false since effects doesn't synthesize such code yet. My "generic const items: support effects" PR (#117530) would change that and rustdoc would handle it correctly (I've double-checked that).
| // R: Resource" | ||
| pub use const_effect_param::lock; | ||
|
|
||
| // Regression test for an issue introduced in PR #116670. |
| @@ -0,0 +1,16 @@ | |||
| #![feature(effects, const_trait_impl)] | |||
There was a problem hiding this comment.
I've renamed the test const-fn.rs to const-effect-param.rs since it's more accurate but git / GitHub doesn't display it as such due to the large number of additional changes in this file.
|
Thanks for the fix! @bors r+ rollup |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#114316 (Add AIX platform support document) - rust-lang#117531 (rustdoc: properly elide cross-crate host effect args) - rust-lang#117650 (Add -Zcross-crate-inline-threshold=yes) - rust-lang#117663 (bump some deps) - rust-lang#117667 (Document clippy_config in nightly-rustc docs) - rust-lang#117698 (Clarify `space_between`) - rust-lang#117700 (coverage: Rename the `run-coverage` test mode to `coverage-run`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#117531 - fmease:rustdoc-effects-properly-elide-x-crate-host-args, r=GuillaumeGomez rustdoc: properly elide cross-crate host effect args Fixes FIXMEs introduced in rust-lang#116670.
…nkov Reenable effects in libcore With rust-lang#116670, rust-lang#117531, and rust-lang#117171, I think we would be comfortable with re-enabling the effects feature for more testing in libcore. r? `@oli-obk` cc `@fmease` cc rust-lang#110395
Fixes FIXMEs introduced in #116670.