Update for new stable and unstable features, and fix a few mistakes.#68
Update for new stable and unstable features, and fix a few mistakes.#68eddyb wants to merge 16 commits intorust-lang:masterfrom
Conversation
CAD97
left a comment
There was a problem hiding this comment.
Everything seems to look good to me.
|
now it's just fixing the CI then we're good to go |
|
CI failure is just a required snapshot update. |
Centril
left a comment
There was a problem hiding this comment.
There are probably more things that need to be tweaked, but here are some corrections.
| ; | ||
|
|
||
| MatchArm = attrs:OuterAttr* "|"? pats:Pat+ % "|" { "if" guard:Expr }? "=>" body:Expr ","?; | ||
| MatchArm = attrs:OuterAttr* "|"? pat:Pat { "if" guard:Expr }? "=>" body:Expr ","?; |
There was a problem hiding this comment.
According to this setup, where nested or-patterns are unstable, then match 0 { 0 | 1 => 0 } is also unstable but it isn't.
There was a problem hiding this comment.
We don't have a proper stability setup, but I can undo this change.
There was a problem hiding this comment.
Maybe just leave a comment instead. You might want to double check if/while let also.
There was a problem hiding this comment.
Also, we annoyingly don't allow fn foo(Ok(x) | Err(x): _); but require parens around the pattern instead.
| | Regular:FnSigInput | ||
| | Regular:{ { pat:Pat ":" }? ty:Type } | ||
| ; | ||
| // unstable(c_variadic): |
There was a problem hiding this comment.
The syntax itself is stable so you can remove this.
| | SelfValue:{ mutable:"mut"? "self" } | ||
| | SelfRef:{ "&" lt:LIFETIME? mutable:"mut"? "self" } |
There was a problem hiding this comment.
The self param is only allowed in the first parameter syntactically (Also, consider lifting this out to its own rule.)
| | CVariadic:FnCVariadicParam | ||
| | RegularAndCVariadic:{ args:FnParam+ % "," "," c_variadic:FnCVariadicParam } |
There was a problem hiding this comment.
... is allowed in any position now syntactically with semantic restrictions in validation to enforce the order (I fixed your FIXME.)
| | Regular:{ { pat:Pat ":" }? ty:Type } | ||
| ; | ||
| // unstable(c_variadic): | ||
| FnCVariadicParam = attrs:OuterAttr* { pat:Pat ":" }? "..."; |
There was a problem hiding this comment.
It's more like this:
FnParamKind =
| SelfParam: FnSelfParam
| Regular:{ { pat:Pat ":" }? ty:FnParamType }
;
FnParamType =
| CVariadic: "..."
| Regular: Type
;| | RangeInclusive:{ | ||
| | start:PatRangeValue { "..." | "..=" } end:PatRangeValue | ||
| // unstable(half_open_range_patterns): | ||
| | { "..." | "..=" } end:PatRangeValue |
There was a problem hiding this comment.
| | { "..." | "..=" } end:PatRangeValue | |
| | "..=" end:PatRangeValue |
| | Slice:{ "[" elems:SlicePatElem* %% "," "]" } | ||
| | Tuple:{ "(" fields:TuplePatField* %% "," ")" } | ||
| // unstable(or_patterns): | ||
| // FIXME(eddyb) find a way to express "2 or more" (like regex `{2,}`). |
There was a problem hiding this comment.
Alternatively you can just explain this as a binary operator rather than a list.
There was a problem hiding this comment.
Fair, it's just weird to think of it as a binary operator because it's fully commutative and associative.
| FnSigInputs = | ||
| | Regular:FnSigInput+ %% "," | ||
| | Variadic:"..." | ||
| | RegularAndVariadic:{ inputs:FnSigInput+ % "," "," "..." } | ||
| | CVariadic:FnSigCVariadicInput | ||
| | RegularAndCVariadic:{ inputs:FnSigInput+ % "," "," c_variadic:FnSigCVariadicInput } | ||
| ; | ||
| FnSigInput = { pat:Pat ":" }? ty:Type; | ||
| FnSigInput = attrs:OuterAttr* { pat:Pat ":" }? ty:Type; | ||
| FnSigCVariadicInput = attrs:OuterAttr* "..."; |
There was a problem hiding this comment.
This isn't right; the actual grammar is defined by:
fn parse_ty_bare_fn(&mut self, generic_params: Vec<GenericParam>) -> PResult<'a, TyKind> {
let unsafety = self.parse_unsafety();
let ext = self.parse_extern()?;
self.expect_keyword(kw::Fn)?;
let decl = self.parse_fn_decl(|_| false, AllowPlus::No)?;
Ok(TyKind::BareFn(P(BareFnTy { ext, unsafety, generic_params, decl })))
}Aside from that false, which adjusts the precedence (bias towards named vs. not), its just the regular FnDecl grammar, and since we don't deal with precedence here, you can use FnDecl instead.
There was a problem hiding this comment.
Well, I'd rather move the full grammar here than have a separate FnDecl, but that makes sense.
So fn(self) parses, I didn't know that.
There was a problem hiding this comment.
Yeah I've made some changes recently in the large scale parser refactorings to simplify things.
There was a problem hiding this comment.
You might want to review the new tests and PR descriptions in:
- parse: unify item parsing & filter illegal item kinds rust#69366
- parse: allow
type Foo: Ordsyntactically rust#69361 - parse: fuse associated and extern items up to defaultness rust#69194
- parse: unify function front matter parsing rust#69023
- parse: towards unified
fngrammar rust#68788 - parser: syntactically allow
selfin allfncontexts rust#68764 - parse: merge
fnsyntax + cleanup item parsing rust#68728 - Merge
TraitItem&ImplItem intoAssocItem` rust#67131
On rust-lang/rust@b9d5ee5 (w/o submodules), out of a total of 13964 files:
I haven't updated
external/rustto rust-lang/rust@b9d5ee5, only locally, let me know if I should.const_generics)c_variadic)unsafeandasync)..) #64 (rest patterns)