-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fragment Specifiers for Generic Arguments #3442
base: master
Are you sure you want to change the base?
Fragment Specifiers for Generic Arguments #3442
Conversation
things instead. | ||
|
||
Also, should `:generic_bound` include the preceding `:` in the match (e.g. `: 'a | ||
+ SomeTrait` in the example above)? And likewise with the `=` before |
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.
don't word-wrap in the middle of a ` code span, it doesn't get rendered properly
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.
Oops, I guess I need a better editor config. Fixed in d77ac90
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.
This RFC does not include specifiers for where-clause, which is highly related to generics. Is this intentional? Maybe we should mention this in text.
|
||
* `:generic`: A full set of generic parameters and their bounds (e.g. `<'a, T: | ||
'a + SomeTrait, const N: usize>`) | ||
* `:generic_param`: A generic parameter (e.g. `'a`, `T`, or `const N`) |
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.
const N
does not work for the macro example above, $param:generic_param $( : $bound:generic_bound )?
. Const generics have the syntax const $param:ident : $ty:ty $(= $default:expr)?
. Note that it is a type instead of generic bounds after the colon, so syntactically it should also accept non-trait-like types like const N: (i32, i32)
, though it's forbidden currently.
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.
This was a deliberate deviation from the default syntax to make it easier to parse an arbitrary set of generic parameters. If we use the fragment specifiers as I defined them in the example macro defined in the RFC (< $( $param:generic_param $( : $bound:generic_bound )? ),+ >
, then this successfully matches <const N: usize>
($param
being const N
and $bound
being usize
.
If it matches as you're requesting that it match, then the same pattern will match this trait, but it will also allow in nonsense like <const N: usize: 'a + SomeTrait>
which my approach avoids. I'm not sure how one could change the pattern to avoid allowing for nonsense like that.
Though also, my approach would match <const N>
, which is also nonsense, so idk which one would be better to reject while matching the macro. I'm open to changing it if you have arguments for your way of splitting the definition between the two fragment specifiers being better.
To your last point, the definition that I give for :generic_bounds
does explicitly allow for a type, so that syntax would be accepted.
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.
Your approach also matches const N: 'a
and 'a: usize
(see my comment below)
* `:generic_bound` matches the | ||
[`TypeParamBounds`](https://doc.rust-lang.org/reference/trait-bounds.html) | ||
(can be the bounds on a type parameter) or | ||
[`LifetimeBounds`](https://doc.rust-lang.org/reference/trait-bounds.html) (can | ||
be the bounds on a lifetime parameter) grammar items, or a type (can be the | ||
bounds on a const parameter). |
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.
Should it be named "generic_bound" or "generic_bounds"?
Also that TypeParamBounds
does not allow empty, while the generic syntax do allow <T: /*nothing*/>
(except for const generics). If we want it to match multiple bounds, it should also match no bounds.
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.
Neat, I never knew that Rust allowed <T:>
as a generic parameter list. That doesn't seem very useful to me, but I'll change it to allow that since I agree that it should be matched if Rust allows it.
I think this would be nice longer-term, but at least for now, the extremely simple case of being able to check for pieces like |
I like the general idea of this RFC, but I don't like that a macro accepting I also don't like that matching all generics syntax with the more granular fragments is pretty verbose: < $(
$( #[$m:meta] )*
$p:generic_param
$( : $( $b:generic_bound )? )?
$( = $d:generic_default )?
),* $(,)? > a where $(
$( $l:lifetime $( : $( $b1:generic_bound )? )? , )?
$( $( for $f:generic )? $t:ty $( : $( $b2:generic_bound )? )? , )?
)* But this isn't forward compatible with
|
Maybe it would be best to remove |
I added a |
Yeah, sadly the only way to prevent it from accepting illegal syntaxes like I tried to strike a happy medium where relatively few illegal syntaxes are allowed (I think they've all been said on this page? Unless I'm missing some) but also it's reasonably non-verbose to parse the full generic parameter definition syntax and we're not adding too many new fragment specifiers into the language, and the split is done in such a way that it could be useful for implementing macros. I'm open to any suggestions you have for making that trade-off better, but this was the best balance between the two goals I could think of. |
When explaining fragment specifiers, this can be explained by adding the new | ||
fragment specifiers and their descriptions: | ||
|
||
* `:generic`: A full set of generic parameters and their bounds (e.g. `<'a, T: |
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.
maybe a nit but generic
being singular confuses me. I think it would make more sense for it to be named generics
or generic_set
, or anything to add plurality to the concept, especially since it would be responsible for parsing the opening and closing angle brackets
I wrote up my thoughts from this rust-internals thread into an RFC.
Rendered