rustdoc: Censor certain complex unevaluated const exprs #98814
Conversation
rustbot
added
the
T-rustdoc
rust-highfive
added
the
S-waiting-on-review
|
I think it should be backported for beta so it'll be in 1.63 stable. |
src/librustdoc/clean/utils.rs
Outdated
| // FIXME: Omit the curly braces if the enclosing expression is an array literal | ||
| // with a repeated element (an `ExprKind::Repeat`) as in such case it | ||
| // would not actually need any disambiguation. | ||
| "{_}".to_owned() |
There was a problem hiding this comment.
Maybe { _ }? (with whitespaces)
src/librustdoc/html/render/mod.rs
Outdated
| // FIXME: `.value()` uses `clean::utils::format_integer_with_underscore_sep` under the | ||
| // hood which adds noisy underscores and a type suffix to number literals. | ||
| // This hurts readability in this context especially when more complex expressions | ||
| // are involved and it doesn't add much of value. | ||
| // Find a way to print constants here without all that jazz. | ||
| write!(w, " = {}", default.value(cx.tcx()).unwrap_or_else(|| default.expr(cx.tcx()))); | ||
| match default.value(cx.tcx()) { | ||
| Some(value) => write!(w, "{}", value), |
There was a problem hiding this comment.
Why only the None is being Escaped?
|
Looks like a nice improvement, thanks for working on this! What does a type with only public fields looks like btw? Could you add a test for this case please? Also cc @notriddle |
fmease
force-pushed
the
minimal-fix-for-issue-97933
branch
from
dcc11ae to
2ad6fb3
Compare
|
I've addressed all of your comments (except for the “typo“ one). |
fmease
force-pushed
the
minimal-fix-for-issue-97933
branch
from
2ad6fb3 to
71b0ed9
Compare
fmease
force-pushed
the
minimal-fix-for-issue-97933
branch
from
71b0ed9 to
d3181a9
Compare
|
I think it's good enough for a first step. Please open issues (if not done already) for the next steps. @bors r+ rollup |
|
📌 Commit d3181a9 has been approved by |
bors
added
S-waiting-on-bors
|
Great! I am going to open issues for things that need to be done. |
|
For the beta nomination, I need other rustdoc people to accept too. Are you ok with backporting this fix bet @rust-lang/rustdoc ? |
…askrgr Rollup of 8 pull requests Successful merges: - rust-lang#98738 (Clarify MIR semantics of checked binary operations) - rust-lang#98782 (Improve spans for specialization error) - rust-lang#98793 (Lint against executable files in the root directory) - rust-lang#98814 (rustdoc: Censor certain complex unevaluated const exprs) - rust-lang#98878 (add more `rustc_pass_by_value`) - rust-lang#98879 (Fix "wrap closure in parenthesis" suggestion for `async` closure) - rust-lang#98886 (incr.comp.: Make split-dwarf commandline options [TRACKED].) - rust-lang#98898 (Add "no-div-regex" eslint rule) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #97933.
This is more of a hotfix for the aforementioned issue. By that, I mean that my proposed patch is
not the best solution but one that does not change as much existing code.
It treats symptoms rather than the root cause.
This PR “censors” certain complex unevaluated constant expressions like
matches, blocks, function calls, struct literals etc. by pretty-printing them as_/{ _ }(number and string literals, paths and()are still printed as one would expect).Resorting to this placeholder is preferable to printing the full expression verbatim since
they can be quite large and verbose resulting in an unreadable mess in the generated documentation.
Further, mindlessly printing the const would leak private and
doc(hidden)struct fields (#97933), at least in the currentstable & nightly implementations which rely on
span_to_snippet(!) andrustc_hir_pretty::id_to_string.The censoring of verbose expressions is probably going to stay longer term.
However, in regards to private and
doc(hidden)struct fields, I have a more proper fix in mindwhich I have already partially implemented locally and for which I am going to open a separate PR sometime soon.
For that, I was already in contact with @GuillaumeGomez.
The proper fix involves rustdoc not falling back on pretty-printing unevaluated consts so easily (what this PR is concerned about)
and instead preferring to print evaluated consts which contain more information allowing it to selectively hide private and
doc(hidden)fields, create hyperlinks etc. generally making the output more granular and precise (compared to the brutal_placeholder).Unfortunately, I was a bit too late and the issue just hit stable (1.62).
Should this be backported to beta or even a potential 1.62.1?
r? @GuillaumeGomez