Merge pull request #12083 from Vexu/c-container-err

parser: add helpful error for C style container declarations

Author: Vexu

lib/std/zig/Ast.zig

@@ -334,6 +334,16 @@ pub fn renderError(tree: Ast, parse_error: Error, stream: anytype) !void {
         .invalid_ampersand_ampersand => {
             return stream.writeAll("ambiguous use of '&&'; use 'and' for logical AND, or change whitespace to ' & &' for bitwise AND");
         },
+        .c_style_container => {
+            return stream.print("'{s} {s}' is invalid", .{
+                parse_error.extra.expected_tag.symbol(), tree.tokenSlice(parse_error.token),
+            });
+        },
+        .zig_style_container => {
+            return stream.print("to declare a container do 'const {s} = {s}'", .{
+                tree.tokenSlice(parse_error.token), parse_error.extra.expected_tag.symbol(),
+            });
+        },
         .previous_field => {
             return stream.writeAll("field before declarations here");
         },
@@ -2541,7 +2551,9 @@ pub const Error = struct {
         expected_initializer,
         mismatched_binary_op_whitespace,
         invalid_ampersand_ampersand,
+        c_style_container,
 
+        zig_style_container,
         previous_field,
         next_field,
 

lib/std/zig/parse.zig

@@ -178,7 +178,6 @@ const Parser = struct {
             .expected_block_or_assignment,
             .expected_block_or_expr,
             .expected_block_or_field,
-            .expected_container_members,
             .expected_expr,
             .expected_expr_or_assignment,
             .expected_fn,
@@ -401,10 +400,12 @@ const Parser = struct {
                                 });
                                 try p.warnMsg(.{
                                     .tag = .previous_field,
+                                    .is_note = true,
                                     .token = last_field,
                                 });
                                 try p.warnMsg(.{
                                     .tag = .next_field,
+                                    .is_note = true,
                                     .token = identifier,
                                 });
                                 // Continue parsing; error will be reported later.
@@ -440,9 +441,15 @@ const Parser = struct {
                     break;
                 },
                 else => {
-                    try p.warn(.expected_container_members);
-                    // This was likely not supposed to end yet; try to find the next declaration.
-                    p.findNextContainerMember();
+                    const c_container = p.parseCStyleContainer() catch |err| switch (err) {
+                        error.OutOfMemory => return error.OutOfMemory,
+                        error.ParseError => false,
+                    };
+                    if (!c_container) {
+                        try p.warn(.expected_container_members);
+                        // This was likely not supposed to end yet; try to find the next declaration.
+                        p.findNextContainerMember();
+                    }
                 },
             }
         }
@@ -978,6 +985,20 @@ const Parser = struct {
             }),
             .keyword_switch => return p.expectSwitchExpr(),
             .keyword_if => return p.expectIfStatement(),
+            .keyword_enum, .keyword_struct, .keyword_union => {
+                const identifier = p.tok_i + 1;
+                if (try p.parseCStyleContainer()) {
+                    // Return something so that `expectStatement` is happy.
+                    return p.addNode(.{
+                        .tag = .identifier,
+                        .main_token = identifier,
+                        .data = .{
+                            .lhs = undefined,
+                            .rhs = undefined,
+                        },
+                    });
+                }
+            },
             else => {},
         }
 
@@ -3466,6 +3487,37 @@ const Parser = struct {
         }
     }
 
+    /// Give a helpful error message for those transitioning from
+    /// C's 'struct Foo {};' to Zig's 'const Foo = struct {};'.
+    fn parseCStyleContainer(p: *Parser) Error!bool {
+        const main_token = p.tok_i;
+        switch (p.token_tags[p.tok_i]) {
+            .keyword_enum, .keyword_union, .keyword_struct => {},
+            else => return false,
+        }
+        const identifier = p.tok_i + 1;
+        if (p.token_tags[identifier] != .identifier) return false;
+        p.tok_i += 2;
+
+        try p.warnMsg(.{
+            .tag = .c_style_container,
+            .token = identifier,
+            .extra = .{ .expected_tag = p.token_tags[main_token] },
+        });
+        try p.warnMsg(.{
+            .tag = .zig_style_container,
+            .is_note = true,
+            .token = identifier,
+            .extra = .{ .expected_tag = p.token_tags[main_token] },
+        });
+
+        _ = try p.expectToken(.l_brace);
+        _ = try p.parseContainerMembers();
+        _ = try p.expectToken(.r_brace);
+        try p.expectSemicolon(.expected_semi_after_decl, true);
+        return true;
+    }
+
     /// Holds temporary data until we are ready to construct the full ContainerDecl AST node.
     /// ByteAlign <- KEYWORD_align LPAREN Expr RPAREN
     fn parseByteAlign(p: *Parser) !Node.Index {

lib/std/zig/parser_test.zig

@@ -212,6 +212,27 @@ test "zig fmt: top-level fields" {
     );
 }
 
+test "zig fmt: C style containers" {
+    try testError(
+        \\struct Foo {
+        \\    a: u32,
+        \\};
+    , &[_]Error{
+        .c_style_container,
+        .zig_style_container,
+    });
+    try testError(
+        \\test {
+        \\    struct Foo {
+        \\        a: u32,
+        \\    };
+        \\}
+    , &[_]Error{
+        .c_style_container,
+        .zig_style_container,
+    });
+}
+
 test "zig fmt: decl between fields" {
     try testError(
         \\const S = struct {

src/Module.zig

@@ -3324,17 +3324,21 @@ pub fn astGenFile(mod: *Module, file: *File) !void {
                 .parent_decl_node = 0,
                 .lazy = .{ .byte_abs = byte_abs },
             }, err_msg, "invalid byte: '{'}'", .{std.zig.fmtEscapes(source[byte_abs..][0..1])});
-        } else if (parse_err.tag == .decl_between_fields) {
-            try mod.errNoteNonLazy(.{
-                .file_scope = file,
-                .parent_decl_node = 0,
-                .lazy = .{ .byte_abs = token_starts[file.tree.errors[1].token] },
-            }, err_msg, "field before declarations here", .{});
-            try mod.errNoteNonLazy(.{
-                .file_scope = file,
-                .parent_decl_node = 0,
-                .lazy = .{ .byte_abs = token_starts[file.tree.errors[2].token] },
-            }, err_msg, "field after declarations here", .{});
+        }
+
+        for (file.tree.errors[1..]) |note| {
+            if (!note.is_note) break;
+
+            try file.tree.renderError(note, msg.writer());
+            err_msg.notes = try mod.gpa.realloc(err_msg.notes, err_msg.notes.len + 1);
+            err_msg.notes[err_msg.notes.len - 1] = .{
+                .src_loc = .{
+                    .file_scope = file,
+                    .parent_decl_node = 0,
+                    .lazy = .{ .byte_abs = token_starts[note.token] },
+                },
+                .msg = msg.toOwnedSlice(),
+            };
         }
 
         {

src/main.zig

@@ -4367,7 +4367,7 @@ fn printErrsMsgToStdErr(
         defer text_buf.deinit();
         const writer = text_buf.writer();
         try tree.renderError(parse_error, writer);
-        const text = text_buf.items;
+        const text = try arena.dupe(u8, text_buf.items);
 
         var notes_buffer: [2]Compilation.AllErrors.Message = undefined;
         var notes_len: usize = 0;
@@ -4388,31 +4388,26 @@ fn printErrsMsgToStdErr(
                 },
             };
             notes_len += 1;
-        } else if (parse_error.tag == .decl_between_fields) {
-            const prev_loc = tree.tokenLocation(0, parse_errors[i + 1].token);
-            notes_buffer[0] = .{
-                .src = .{
-                    .src_path = path,
-                    .msg = "field before declarations here",
-                    .byte_offset = @intCast(u32, prev_loc.line_start),
-                    .line = @intCast(u32, prev_loc.line),
-                    .column = @intCast(u32, prev_loc.column),
-                    .source_line = tree.source[prev_loc.line_start..prev_loc.line_end],
-                },
-            };
-            const next_loc = tree.tokenLocation(0, parse_errors[i + 2].token);
-            notes_buffer[1] = .{
+        }
+
+        for (parse_errors[i + 1 ..]) |note| {
+            if (!note.is_note) break;
+
+            text_buf.items.len = 0;
+            try tree.renderError(note, writer);
+            const note_loc = tree.tokenLocation(0, note.token);
+            notes_buffer[notes_len] = .{
                 .src = .{
                     .src_path = path,
-                    .msg = "field after declarations here",
-                    .byte_offset = @intCast(u32, next_loc.line_start),
-                    .line = @intCast(u32, next_loc.line),
-                    .column = @intCast(u32, next_loc.column),
-                    .source_line = tree.source[next_loc.line_start..next_loc.line_end],
+                    .msg = try arena.dupe(u8, text_buf.items),
+                    .byte_offset = @intCast(u32, note_loc.line_start),
+                    .line = @intCast(u32, note_loc.line),
+                    .column = @intCast(u32, note_loc.column),
+                    .source_line = tree.source[note_loc.line_start..note_loc.line_end],
                 },
             };
-            notes_len = 2;
-            i += 2;
+            i += 1;
+            notes_len += 1;
         }
 
         const extra_offset = tree.errorOffset(parse_error);