Cleanup round 2 from review

hryx · hryx · commit ec124a4ab2bf · 2019-05-11T16:07:06.000-07:00
- Remove TODO about stage1 CompareExpr and make a follow-up task - Support enum literals, update grammar, and add test - Remove TODO comments about enum literals - Remove TODO question about ErrorSetDecl not hard-failing - Error if label found but no block - Disallow duplicate pointer qualifiers (const, allowzero, etc.) and add a stage2 compile error test (disabled; see ziglang#1364)
diff --git a/std/zig/parse.zig b/std/zig/parse.zig
@@ -808,7 +808,6 @@ fn parseBoolAndExpr(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node
 // CompareExpr <- BitwiseExpr (CompareOp BitwiseExpr)?
 fn parseCompareExpr(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node {
     return parseBinOpExpr(arena, it, tree, parseCompareOp, parseBitwiseExpr, .Once);
-    // TODO: stage1 supplies BinOpChainInf, not Once, but grammar uses `?`
 }
 
 // BitwiseExpr <- BitShiftExpr (BitwiseOp BitShiftExpr)*
@@ -849,8 +848,6 @@ fn parsePrefixExpr(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node {
 //      / Block
 //      / CurlySuffixExpr
 fn parsePrimaryExpr(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node {
-    // TODO: enum literal not represented in grammar: https://github.com/ziglang/zig/issues/2235
-    if (try parseEnumLiteral(arena, it, tree)) |node| return node;
     if (try parseAsmExpr(arena, it, tree)) |node| return node;
     if (try parseIfExpr(arena, it, tree)) |node| return node;
 
@@ -1213,6 +1210,7 @@ fn parseSuffixExpr(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node {
 //     <- BUILTINIDENTIFIER FnCallArguments
 //      / CHAR_LITERAL
 //      / ContainerDecl
+//      / DOT IDENTIFIER
 //      / ErrorSetDecl
 //      / FLOAT
 //      / FnProto
@@ -1243,6 +1241,7 @@ fn parsePrimaryTypeExpr(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*N
         return &node.base;
     }
     if (try parseContainerDecl(arena, it, tree)) |node| return node;
+    if (try parseEnumLiteral(arena, it, tree)) |node| return node;
     if (try parseErrorSetDecl(arena, it, tree)) |node| return node;
     if (try parseFloatLiteral(arena, it, tree)) |node| return node;
     if (try parseFnProto(arena, it, tree)) |node| return node;
@@ -1318,7 +1317,7 @@ fn parseContainerDecl(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Nod
 fn parseErrorSetDecl(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node {
     const error_token = eatToken(it, .Keyword_error) orelse return null;
     if (eatToken(it, .LBrace) == null) {
-        // TODO: Check the order of choices that leads to this not being "expectToken"
+        // Might parse as `KEYWORD_error DOT IDENTIFIER` later in PrimaryTypeExpr, so don't error
         _ = prevToken(it);
         return null;
     }
@@ -1381,8 +1380,11 @@ fn parseLabeledTypeExpr(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*N
     }
 
     if (label != null) {
-        _ = prevToken(it); // rewind IDENTIFIER
-        _ = prevToken(it); // rewind ":"
+        // If we saw a label, there should have been a block next
+        try tree.errors.push(AstError{
+            .ExpectedLBrace = AstError.ExpectedLBrace{ .token = it.index },
+        });
+        return Error.UnexpectedToken;
     }
     return null;
 }
@@ -1519,11 +1521,10 @@ fn parseAsmExpr(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node {
     return &node.base;
 }
 
-// TODO: enum literal not represented in grammar: https://github.com/ziglang/zig/issues/2235
+// DOT IDENTIFIER
 fn parseEnumLiteral(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node {
     const dot = eatToken(it, .Period) orelse return null;
     const name = try expectToken(it, tree, .Identifier);
-
     const node = try arena.create(Node.EnumLiteral);
     node.* = Node.EnumLiteral{
         .base = Node{ .id = .EnumLiteral },
@@ -1646,7 +1647,12 @@ fn parseBlockLabel(arena: *Allocator, it: *TokenIterator, tree: *Tree) ?TokenInd
 fn parseFieldInit(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node {
     const period_token = eatToken(it, .Period) orelse return null;
     const name_token = try expectToken(it, tree, .Identifier);
-    const eq_token = try expectToken(it, tree, .Equal);
+    const eq_token = eatToken(it, .Equal) orelse {
+        // `.Name` may also be an enum literal, which is a later rule.
+        _ = prevToken(it); // rewind IDENTIFIER
+        _ = prevToken(it); // rewind DOT
+        return null;
+    };
     const expr_node = try expectNode(arena, it, tree, parseExpr, AstError{
         .ExpectedExpr = AstError.ExpectedExpr{ .token = it.index },
     });
@@ -2204,10 +2210,8 @@ fn parsePrefixOp(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node {
     return &node.base;
 }
 
-// TODO: last choice allows for `*const volatile volatile const`, `*align(4) align(8) align(4)` etc.
 // TODO: ArrayTypeStart is either an array or a slice, but const/allowzero only work on
-//       pointers. (Note: does align(x) work for arrays as well as slices/pointers? Does volatile?)
-//       Maybe it would be better to have two separate rules?
+//       pointers. Consider updating this rule:
 //       ...
 //       / ArrayTypeStart
 //       / SliceTypeStart (ByteAlign / KEYWORD_const / KEYWORD_volatile / KEYWORD_allowzero)*
@@ -2255,23 +2259,48 @@ fn parsePrefixTypeOp(arena: *Allocator, it: *TokenIterator, tree: *Tree) !?*Node
         switch (node.cast(Node.PrefixOp).?.op) {
             .ArrayType => {},
             .SliceType => |*slice_type| {
+                // Collect pointer qualifiers in any order, but disallow duplicates
                 while (true) {
                     if (try parseByteAlign(arena, it, tree)) |align_expr| {
+                        if (slice_type.align_info != null) {
+                            try tree.errors.push(AstError{
+                                .ExtraAlignQualifier = AstError.ExtraAlignQualifier{ .token = it.index },
+                            });
+                            return Error.UnexpectedToken;
+                        }
                         slice_type.align_info = Node.PrefixOp.PtrInfo.Align{
                             .node = align_expr,
                             .bit_range = null,
                         };
                         continue;
                     }
                     if (eatToken(it, .Keyword_const)) |const_token| {
+                        if (slice_type.const_token != null) {
+                            try tree.errors.push(AstError{
+                                .ExtraConstQualifier = AstError.ExtraConstQualifier{ .token = it.index },
+                            });
+                            return Error.UnexpectedToken;
+                        }
                         slice_type.const_token = const_token;
                         continue;
                     }
                     if (eatToken(it, .Keyword_volatile)) |volatile_token| {
+                        if (slice_type.volatile_token != null) {
+                            try tree.errors.push(AstError{
+                                .ExtraVolatileQualifier = AstError.ExtraVolatileQualifier{ .token = it.index },
+                            });
+                            return Error.UnexpectedToken;
+                        }
                         slice_type.volatile_token = volatile_token;
                         continue;
                     }
                     if (eatToken(it, .Keyword_allowzero)) |allowzero_token| {
+                        if (slice_type.allowzero_token != null) {
+                            try tree.errors.push(AstError{
+                                .ExtraAllowZeroQualifier = AstError.ExtraAllowZeroQualifier{ .token = it.index },
+                            });
+                            return Error.UnexpectedToken;
+                        }
                         slice_type.allowzero_token = allowzero_token;
                         continue;
                     }
diff --git a/std/zig/parser_test.zig b/std/zig/parser_test.zig
@@ -12,6 +12,21 @@ test "zig fmt: enum literal" {
     );
 }
 
+test "zig fmt: enum literal inside array literal" {
+    try testCanonical(
+        \\test "enums in arrays" {
+        \\    var colors = []Color{.Green};
+        \\    colors = []Colors{ .Green, .Cyan };
+        \\    colors = []Colors{
+        \\        .Grey,
+        \\        .Green,
+        \\        .Cyan,
+        \\    };
+        \\}
+        \\
+    );
+}
+
 test "zig fmt: character literal larger than u8" {
     try testCanonical(
         \\const x = '\U01f4a9';
diff --git a/test/stage2/compile_errors.zig b/test/stage2/compile_errors.zig
@@ -27,4 +27,28 @@ pub fn addCases(ctx: *TestContext) !void {
         \\    return 36893488147419103232;
         \\}
     , "1.zig", 2, 12, "integer value '36893488147419103232' cannot be stored in type 'c_int'");
+
+    try ctx.testCompileError(
+        \\comptime {
+        \\    var a: *align(4) align(4) i32 = 0;
+        \\}
+    , "1.zig", 2, 22, "Extra align qualifier");
+
+    try ctx.testCompileError(
+        \\comptime {
+        \\    var b: *const const i32 = 0;
+        \\}
+    , "1.zig", 2, 19, "Extra align qualifier");
+
+    try ctx.testCompileError(
+        \\comptime {
+        \\    var c: *volatile volatile i32 = 0;
+        \\}
+    , "1.zig", 2, 22, "Extra align qualifier");
+
+    try ctx.testCompileError(
+        \\comptime {
+        \\    var d: *allowzero allowzero i32 = 0;
+        \\}
+    , "1.zig", 2, 23, "Extra align qualifier");
 }
