-
Notifications
You must be signed in to change notification settings - Fork 1.6k
#[doc(consts)] #3770
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
base: master
Are you sure you want to change the base?
#[doc(consts)] #3770
Conversation
text/0000-doc-consts.md
Outdated
|
||
The `#[doc(consts)]` attribute can be placed on any item to control how contained constant expressions are displayed in rustdoc-generated documentation. | ||
|
||
* `#[doc(consts = "fold")]` will show them in their fully-evaluated state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about "eval"
for fully evaluated item and "as-is
" for item as written?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"constant folding" is a well established term used for this exact process, and "eval" is much more general (future display modes would likely also involve evaluating the expression).
on the other hand, i don't feel strongly about "expr"
, but my second choice would probably be "verbatim"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Rust ever refer to CTFE as constant folding? The docs use evaluation https://doc.rust-lang.org/reference/const_eval.html, and to me "const folding" is an identify+transform optimization pass (as opposed to compile-time evaluation that must unconditionally be run).
Different crates and items have conflicting requirements for their constants. | ||
For some, [the exact value of a constant is platform dependant](https://internals.rust-lang.org/t/pre-rfc-doc-consts-attribute/21987/9). | ||
For others, [constant folding obsurces the meaning of values](https://github.com/rust-lang/rust/issues/128347). | ||
Hovever, [showing a constant as written may leak implementation details], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a link missing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's one of the linked issues in rust-lang/rust#99688 (i'll go dig through it in a bit)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it, we actually want to have a similar attribute for types:
- A subgroup of users would like rustdoc to normalize types. Type normalization almost became the default (Normalize
<X as Y>::T
for rustdoc rust#77467) but due to limitations of the current (but not next-gen) trait solver (namely: fatal overflows), it had to be gated behind the-Znormalize-docs
where this behavior currently resides. Also, rustdoc is notoriously bad at properly tracking binders (think higher-ranked lifetimes). It's gotten better but it's not sufficient (cc useObligationCtxt
notQueryNormalizer
in rustdoc'snormalization
rust#108503). - A subgroup of users would like public type aliases to remain unexpanded and private ones to be folded away. This is what rustdoc currently does for items of the local crate. Infamously, this does not extend to inlined cross-crate re-exports (rustdoc: reexported type aliases are not preserved in function signatures rust#15823) as they use a different IR (middle IR vs HIR). This has created the multi-year effort of representing type aliases in the "type/trait system IR" (aka middle::ty). See Tracking issue for lazy type aliases rust#112792.
However, [1] (type normalization) is inherently incompatible with [2] (public type alias preservation). I'm currently driving [2] and will block any attempt at stabilizing [1] :P (not that there was any push to do so). This screams "we'd like to have configuration".
@fmease That's a very good point, it would be nice to have some syntax that encapsulates both. Perhaps a syntax like |
@fmease added a |
|
||
* `#[doc(normalize::consts = "fold")]` will show them in their fully-evaluated state. | ||
* `#[doc(normalize::consts = "expr")]` will show them as-written. | ||
* `#[doc(normalize::consts = "hide")]` will cause constant expressions to be replaced with `_` or not shown at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why use the qualified name syntax rather than something more normal (😉) like
#[doc(normalize(consts = "expr", types = "hide"))]
static X: [SomeComplicatedType; 123456] = evaluate_lookup_table(include_bytes!("lookup.bin"));
// or
#[doc(normalize_consts = "expr")]
#[doc(normalize_types = "hide")]
static X: [SomeComplicatedType; 123456] = evaluate_lookup_table(include_bytes!("lookup.bin"));
AFAIK in an attribute the x::y
syntax is only1 used for "tool attributes" #[rustfmt::skip]
and "tool lints" #[warn(clippy::pedantic)]
, and normalize
here is certainly not a "tool".
Footnotes
-
unless when that
x::y
is used to refer to an item like in#[cfg(accessible(x::y))]
, of course ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the diagnostic
namespace exists. also this isn't a full attribute, it's an argument to the doc
attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes and clippy, rustfmt, diagnostic, miri, rust_analyzer
are all regarded as "tools".
The point is the x::y
syntax appearing in built-in Rust attributes only as
- tool attributes/lints
#[cfg(accessible(crate::item))]
#[instruction_set(arm::a32)]
/#[instruction_set(arm::t32)]
#[doc(normalize::consts)]
fit into neither category, so this RFC is introducing a 4th item. Comparing to the existing three:
- for tool attributes/lints, it is natural to assign each tool into its own "namespace", and the namespace is extensible outside of the Rust Team's control (clippy, rustfmt, etc.)
- for
cfg_accessible
, the path syntax is just natural - for
instruction_set
, it is also natural1 to use the architecture name as the "namespace" to group the sub-instruction-sets, and again the namespace is extensible (arm, mips, etc.) in a way not dictated by the Rust.
In this RFC there's nothing suggests normalize
needs to be namespace-like. There is not a second #[doc(something::a = "b")]
similar to #[doc(normalize::x = "y")]
in terms of categorization. The handling of #[doc(...)]
is managed by wholly by rustdoc, there is no 3rd party involvement. There is no argument why non-namespaced syntax like #[doc(normalize(x = "y"))]
is sub-optimal. (Also I don't see anywhere in @fmease
's comment suggesting turning normalize
into a prefix, they is just suggesting to consider normalization of types as well.)
In summary, pivoting to #[doc(normalize::consts = "fold")]
is an abuse of syntax for zero good reasons. 👎.
Footnotes
-
though I'd argue that this is not really a good syntax to specify the instruction set, as RFC: Add a new
#[instruction_set(...)]
attribute for supporting per-function instruction set changes #2867 suggested the feature may be extended to support 65c02 mode in 65c816, if Rust ever gained support of it, but both65c816
and65c02
aren't valid identifiers (the llvm-mos project calls the architecture "mos" so it's not an actual issue). ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I simply believe that the nested function call syntax is less readable, and like you said, rustdoc has complete control over the doc(
attribute syntax, so why not use the more readable syntax?
Personally, I would've preferred if all doc(
attributes were instead written as doc::
, I think that would've made more sense.
I think instruction_set
actually acts as pretty good precedence for this, after all, it could've easily been written as #[instruction_set(arm(a32))]
.
I think I'll wait for a third party to weigh in before changing the proposed syntax again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I simply believe that the nested function call syntax is less readable, and like you said, rustdoc has complete control over the
doc(
attribute syntax, so why not use the more readable syntax?
I just don't see how #[doc(normalize::consts = "x")]
is any more readable then #[doc(normalize_consts = "x")]
. Even #[doc(normalize(consts = "x"))]
is not really that bad as the nesting must be limited at level 2, unlike #![doc(test(attr(deny(dead_code))))]
.
The slight advantage of the nested syntax is allowing the two normalization instructions grouped into a single attribute, #[doc(normalize(consts = "x", types = "y"))]
, but breaking it into 2 lines #[doc(normalize_consts = "x")] #[doc(normalize_types = "y")]
isn't really a big disadvantage IMO.
I think
instruction_set
actually acts as pretty good precedence for this, after all, it could've easily been written as#[instruction_set(arm(a32))]
.
#2867 starts out as #[instruction_set(a32)]
, the arm
namespace was added on later for disambiguation.
The original suggestion was #[instruction_set(arm_a32)]
(not the "real name", thus hurts searchability) and then #[instruction_set(arm, a32)]
(does not suggest a hierarchical relationship).
None of the arguments are relevant to normalize::consts
/ normalize::types
.
Rendered