Update for new stable and unstable features, and fix a few mistakes. #68

eddyb · 2020-03-29T20:45:08Z

On rust-lang/rust@b9d5ee5 (w/o submodules), out of a total of 13964 files:

Before	After	Δ
1043	1045	+2	parsed fully and unambiguously
12221	12534	+313	parsed fully (but ambiguously)
613	298	-315	parsed partially (only a prefix)
87	87	0	didn't parse at all (lexer error?)

I haven't updated external/rust to rust-lang/rust@b9d5ee5, only locally, let me know if I should.

closes Add unstable const generics #43 (const_generics)
closes Add unstable C variadic function argument #45 (c_variadic)
closes Add unstable attributes on function arguments #46 (parameter attributes)
closes Struct pattern fields should allow outer attributes. #57 (struct pattern field attributes)
closes Swap unsafe/async #62 (swapping unsafe and async)
closes Account for rest patterns (..) #64 (rest patterns)

….) types.

CAD97

Everything seems to look good to me.

qmx

awesome

qmx · 2020-03-29T21:00:16Z

now it's just fixing the CI then we're good to go

CAD97 · 2020-03-29T21:31:22Z

CI failure is just a required snapshot update.

Centril

There are probably more things that need to be tweaked, but here are some corrections.

Centril · 2020-03-30T08:30:54Z

grammar/expr.lyg

@@ -125,6 +127,6 @@ ElseExpr =
  | If:If
  ;

-MatchArm = attrs:OuterAttr* "|"? pats:Pat+ % "|" { "if" guard:Expr }? "=>" body:Expr ","?;
+MatchArm = attrs:OuterAttr* "|"? pat:Pat { "if" guard:Expr }? "=>" body:Expr ","?;


According to this setup, where nested or-patterns are unstable, then match 0 { 0 | 1 => 0 } is also unstable but it isn't.

We don't have a proper stability setup, but I can undo this change.

Maybe just leave a comment instead. You might want to double check if/while let also.

Also, we annoyingly don't allow fn foo(Ok(x) | Err(x): _); but require parens around the pattern instead.

Centril · 2020-03-30T08:33:53Z

grammar/item.lyg

  ;
+// unstable(c_variadic):


The syntax itself is stable so you can remove this.

Centril · 2020-03-30T08:34:57Z

grammar/item.lyg

  | SelfValue:{ mutable:"mut"? "self" }
  | SelfRef:{ "&" lt:LIFETIME? mutable:"mut"? "self" }


The self param is only allowed in the first parameter syntactically (Also, consider lifting this out to its own rule.)

Centril · 2020-03-30T08:36:31Z

grammar/item.lyg

+  | CVariadic:FnCVariadicParam
+  | RegularAndCVariadic:{ args:FnParam+ % "," "," c_variadic:FnCVariadicParam }


... is allowed in any position now syntactically with semantic restrictions in validation to enforce the order (I fixed your FIXME.)

Centril · 2020-03-30T08:43:20Z

grammar/item.lyg

  ;
+// unstable(c_variadic):
+FnCVariadicParam = attrs:OuterAttr* { pat:Pat ":" }? "...";


It's more like this:

FnParamKind = | SelfParam: FnSelfParam | Regular:{ { pat:Pat ":" }? ty:FnParamType } ; FnParamType = | CVariadic: "..." | Regular: Type ;

grammar/pat.lyg

Centril · 2020-03-30T08:48:45Z

grammar/pat.lyg

+  | RangeInclusive:{
+    | start:PatRangeValue { "..." | "..=" } end:PatRangeValue
+    // unstable(half_open_range_patterns):
+    | { "..." | "..=" } end:PatRangeValue


Suggested change

| { "..." | "..=" } end:PatRangeValue

| "..=" end:PatRangeValue

Centril · 2020-03-30T08:49:38Z

grammar/pat.lyg

-  | Slice:{ "[" elems:SlicePatElem* %% "," "]" }
-  | Tuple:{ "(" fields:TuplePatField* %% "," ")" }
+  // unstable(or_patterns):
+  // FIXME(eddyb) find a way to express "2 or more" (like regex `{2,}`).


Alternatively you can just explain this as a binary operator rather than a list.

Fair, it's just weird to think of it as a binary operator because it's fully commutative and associative.

Centril · 2020-03-30T08:55:32Z

grammar/type.lyg

 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* "...";


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.

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.

Yeah I've made some changes recently in the large scale parser refactorings to simplify things.

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: Ord syntactically rust#69361

parse: fuse associated and extern items up to defaultness rust#69194

parse: unify function front matter parsing rust#69023

parse: towards unified fn grammar rust#68788

parser: syntactically allow self in all fn contexts rust#68764

parse: merge fn syntax + cleanup item parsing rust#68728

Merge TraitItem & ImplItem into AssocItem` rust#67131

eddyb added 16 commits March 28, 2020 15:04

Add .vscode to .gitignore.

dd1f35c

Update gll dependency.

db0a296

pat: support rest patterns, or_patterns and half_open_range_patterns.

7936b13

generics: support associated_type_bounds.

72d5a0a

generics: support const_generics.

82efb69

item: support function parameter attributes.

c815b25

item: swap "async" and "unsafe" in FnHeader.

3d773d9

item: support c_variadic.

159762b

item: support const_trait_impl.

d3d3d02

generics: support const_trait_bound_opt_out.

d3aa7da

expr: support raw_ref_op.

62017c0

coverage: strip shebangs.

2c34092

item: fix TraitAlias syntax.

eeeae03

pat: allow attributes in StructPatField.

4ede4ea

item: fix Trait to allow inner attributes.

152276a

item,expr,type: generalize parameter attributes to closures and fn(..…

5560302

….) types.

CAD97 approved these changes Mar 29, 2020

View reviewed changes

qmx approved these changes Mar 29, 2020

View reviewed changes

Centril reviewed Mar 30, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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. #68

eddyb commented Mar 29, 2020 •

edited

Loading

CAD97 left a comment

qmx left a comment

qmx commented Mar 29, 2020

CAD97 commented Mar 29, 2020

Centril left a comment

Centril Mar 30, 2020

eddyb Mar 30, 2020

Centril Mar 30, 2020

Centril Mar 30, 2020

Centril Mar 30, 2020

Centril Mar 30, 2020

Centril Mar 30, 2020

Centril Mar 30, 2020 •

edited

Loading

Centril Mar 30, 2020

Centril Mar 30, 2020

eddyb Mar 30, 2020

Centril Mar 30, 2020

eddyb Mar 30, 2020

Centril Mar 30, 2020

Centril Mar 30, 2020 •

edited

Loading

		\| SelfValue:{ mutable:"mut"? "self" }
		\| SelfRef:{ "&" lt:LIFETIME? mutable:"mut"? "self" }

		\| CVariadic:FnCVariadicParam
		\| RegularAndCVariadic:{ args:FnParam+ % "," "," c_variadic:FnCVariadicParam }

	\| { "..." \| "..=" } end:PatRangeValue
	\| "..=" end:PatRangeValue

Update for new stable and unstable features, and fix a few mistakes. #68

Are you sure you want to change the base?

Update for new stable and unstable features, and fix a few mistakes. #68

Conversation

eddyb commented Mar 29, 2020 • edited Loading

CAD97 left a comment

Choose a reason for hiding this comment

qmx left a comment

Choose a reason for hiding this comment

qmx commented Mar 29, 2020

CAD97 commented Mar 29, 2020

Centril left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Centril Mar 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Centril Mar 30, 2020 • edited Loading

Choose a reason for hiding this comment

eddyb commented Mar 29, 2020 •

edited

Loading

Centril Mar 30, 2020 •

edited

Loading

Centril Mar 30, 2020 •

edited

Loading