[rustdoc-json] Make header a vec of modifiers, and FunctionPointer consistent
#81891
Conversation
…h Function and Method.
rustbot
added
A-rustdoc-json
rust-highfive
added
the
S-waiting-on-review
| @@ -108,7 +108,7 @@ def check_type(ty): | |||
| elif ty["kind"] == "function_pointer": | |||
| for param in ty["inner"]["generic_params"]: | |||
| check_generic_param(param) | |||
| check_decl(ty["inner"]["inner"]) | |||
| check_decl(ty["inner"]["decl"]) | |||
There was a problem hiding this comment.
Previous implementation looks wrong, other call(s) of check_decl are ["inner"]["decl"], and it doesn't look like there should ever be an ["inner"]["inner"] in the output.
| @@ -281,19 +281,28 @@ pub enum StructType { | |||
| Unit, | |||
| } | |||
|
|
|||
| #[non_exhaustive] | |||
There was a problem hiding this comment.
Other things may be added as modifiers in the language, or default on methods could be interpreted as a modifier. This way things can be added without it being breaking
There was a problem hiding this comment.
One detail with the addition of new variants would be that the order is not quite arbitrary. The grammar apppears to be currently:
AsyncConstQualifiers? unsafe? (extern Abi?)?
So while technically permitting the forward compatible addition of new qualifiers, a pretty-printer might still be confused by their order and not produce syntactically valid method/type declarations.
There was a problem hiding this comment.
True. Joshua said that we should use sets over vecs specifically so order is not guaranteed as part of the contract. I think that plus non_exhaustive means it's at least fairly obvious that we're not guaranteeing anything (all matches will require a _ part), the only issue is if people think we're implying something about the syntax.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
LGTM but I'd like to have @HeroicKatora to take a look if they have time.
jyn514
added
S-waiting-on-author
|
Note: I considered and personally rejected instead using a struct with boolean flags. There were two main reasons:
|
This comment has been minimized.
This comment has been minimized.
|
@bors r+ |
|
📌 Commit be4ea06 has been approved by |
bors
added
S-waiting-on-bors
Rollup of 11 pull requests Successful merges: - rust-lang#80523 (#[doc(inline)] sym_generated) - rust-lang#80920 (Visit more targets when validating attributes) - rust-lang#81720 (Updated smallvec version due to RUSTSEC-2021-0003) - rust-lang#81891 ([rustdoc-json] Make `header` a vec of modifiers, and FunctionPointer consistent) - rust-lang#81912 (Implement the precise analysis pass for lint `disjoint_capture_drop_reorder`) - rust-lang#81914 (Fixing bad suggestion for `_` in `const` type when a function rust-lang#81885) - rust-lang#81919 (BTreeMap: fix internal comments) - rust-lang#81927 (Add a regression test for rust-lang#32498) - rust-lang#81965 (Fix MIR pretty printer for non-local DefIds) - rust-lang#82029 (Use debug log level for developer oriented logs) - rust-lang#82056 (fix ice (rust-lang#82032)) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Bumps version number and adds tests, this is a breaking change. I can split this into two (
is_unsafe->headerandheader: Vec<Modifiers>) if desired.Rationale: Modifiers are individual notes on a function, it makes more sense for them to be a list of an independent enum over a String which is inconsistently exposing the HIR representation (prefix_str vs custom literals).
Function pointers currently only support
unsafe, but there has been talk on and off about allowing them to also supportconst, and this makes handling their modifiers consistent with handling those of a function, allowing better shared code.@rustbot modify labels: +A-rustdoc-json +T-rustdoc
CC: @HeroicKatora
r? @jyn514