From 3681c7c506829322a2980d5bd98e493968fdea68 Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Fri, 3 Jul 2026 13:22:52 +0200 Subject: [PATCH] std.zig, grammar: agree on pointer modifier parsing Currently the grammar only allows exactly one pointer modifier order but the Parse.zig implementation allows any order. However, the current Parse.zig behavior of allowing any order while forbidding duplicate modifiers would require a combinatorial explosion of grammar rules to specify (currently 5! i.e. 120). The simplest, most permissive grammar would remove the ordering requirement and allow duplicate pointer modifiers. However, this would unfortunately require a more complex AST data layout due to the child nodes of align() and addrspace() modifiers. Since these are the only pointer modifiers with child nodes and 2! is a perfectly reasonable number, forbid duplicate align() and addrspace() modifiers to keep the Ast data structure simple. In order to resolve the difference between the formal grammar and parser implementation, allow duplicate single-token modifiers (allowzero, const, volatile) in the grammar and move the compile error for that case to AstGen. Note that zig fmt will now silently remove duplicate single-token modifiers, which I think is a nice little UX improvement. --- doc/langref/grammar.peg | 23 +++++++- lib/std/zig/Ast.zig | 35 ++++++------ lib/std/zig/AstGen.zig | 3 ++ lib/std/zig/Parse.zig | 50 ++++++++--------- lib/std/zig/parser_fuzz.zig | 5 ++ lib/std/zig/parser_generated_oracle.zig | 72 ++++++++++++++++++++++++- lib/std/zig/parser_test.zig | 4 +- 7 files changed, 143 insertions(+), 49 deletions(-) diff --git a/doc/langref/grammar.peg b/doc/langref/grammar.peg index a571b430aa06c231d1f82c8faf68d29b533e7a15..d52e432ef70c5ef810fdd592f9817c24134b7332 100644 --- a/doc/langref/grammar.peg +++ b/doc/langref/grammar.peg @@ -364,10 +364,29 @@ PrefixOp PrefixTypeOp <- QUESTIONMARK / KEYWORD_anyframe MINUSRARROW - / (ManyPtrTypeStart / SliceTypeStart) KEYWORD_allowzero? ByteAlign? AddrSpace? KEYWORD_const? KEYWORD_volatile? - / SinglePtrTypeStart KEYWORD_allowzero? BitAlign? AddrSpace? KEYWORD_const? KEYWORD_volatile? + / (ManyPtrTypeStart / SliceTypeStart) PtrMods + / SinglePtrTypeStart SinglePtrMods / ArrayTypeStart +# Forbid more than one align or addrspace pointer modifier since these +# modifiers contain sub-expressions. This allows for a simpler AST data layout. +# Allow duplicate single-token modifiers (allowzero/const/volatile) in the grammar +# to avoid the combinatorial explosion of grammar rules necessary to forbid duplicates +# while permitting arbitrary order. A compile error for duplicates single-token modifiers +# is emitted during "AstGen" after parsing is complete. +PtrMods + <- PtrMod* ByteAlign? PtrMod* AddrSpace? PtrMod* + / PtrMod* AddrSpace? PtrMod* ByteAlign? PtrMod* + +SinglePtrMods + <- PtrMod* BitAlign? PtrMod* AddrSpace? PtrMod* + / PtrMod* AddrSpace? PtrMod* BitAlign? PtrMod* + +PtrMod + <- KEYWORD_allowzero + / KEYWORD_const + / KEYWORD_volatile + PrefixTypeOpPrefix <- QUESTIONMARK / KEYWORD_anyframe MINUSRARROW diff --git a/lib/std/zig/Ast.zig b/lib/std/zig/Ast.zig index 260f62abbb0871a1332978640b1b675a9c2156e9..abd0170803f330cc2e0a51e86a07c718a783326d 100644 --- a/lib/std/zig/Ast.zig +++ b/lib/std/zig/Ast.zig @@ -465,15 +465,6 @@ pub fn renderError(tree: Ast, parse_error: Error, w: *Writer) Writer.Error!void .extra_align_qualifier => { return w.writeAll("extra align qualifier"); }, - .extra_allowzero_qualifier => { - return w.writeAll("extra allowzero qualifier"); - }, - .extra_const_qualifier => { - return w.writeAll("extra const qualifier"); - }, - .extra_volatile_qualifier => { - return w.writeAll("extra volatile qualifier"); - }, .ptr_mod_on_array_child_type => { return w.print("pointer modifier '{s}' not allowed on array child type", .{ tree.tokenTag(parse_error.token).symbol(), @@ -2110,6 +2101,7 @@ fn fullPtrTypeComponents(tree: Ast, info: full.PtrType.Components) full.PtrType .allowzero_token = null, .const_token = null, .volatile_token = null, + .duplicate_token = null, .ast = info, }; // We need to be careful that we don't iterate over any sub-expressions @@ -2125,9 +2117,24 @@ fn fullPtrTypeComponents(tree: Ast, info: full.PtrType.Components) full.PtrType const end = tree.firstToken(info.child_type); while (i < end) : (i += 1) { switch (tree.tokenTag(i)) { - .keyword_allowzero => result.allowzero_token = i, - .keyword_const => result.const_token = i, - .keyword_volatile => result.volatile_token = i, + .keyword_allowzero => { + if (result.allowzero_token != null) { + result.duplicate_token = i; + } + result.allowzero_token = i; + }, + .keyword_const => { + if (result.const_token != null) { + result.duplicate_token = i; + } + result.const_token = i; + }, + .keyword_volatile => { + if (result.volatile_token != null) { + result.duplicate_token = i; + } + result.volatile_token = i; + }, .keyword_align => { if (info.bit_range_end.unwrap()) |bit_range_end| { assert(info.bit_range_start != .none); @@ -2724,6 +2731,7 @@ pub const full = struct { allowzero_token: ?TokenIndex, const_token: ?TokenIndex, volatile_token: ?TokenIndex, + duplicate_token: ?TokenIndex, ast: Components, pub const Components = struct { @@ -2857,9 +2865,6 @@ pub const Error = struct { extern_fn_body, extra_addrspace_qualifier, extra_align_qualifier, - extra_allowzero_qualifier, - extra_const_qualifier, - extra_volatile_qualifier, ptr_mod_on_array_child_type, invalid_bit_range, same_line_doc_comment, diff --git a/lib/std/zig/AstGen.zig b/lib/std/zig/AstGen.zig index e41536f7d456fb0ccf8397742558971d0e5ce7d4..a849f790dd92861b7dd7659a087edec096e9a650 100644 --- a/lib/std/zig/AstGen.zig +++ b/lib/std/zig/AstGen.zig @@ -3745,6 +3745,9 @@ fn ptrType( if (ptr_info.size == .c and ptr_info.allowzero_token != null) { return gz.astgen.failTok(ptr_info.allowzero_token.?, "C pointers always allow address zero", .{}); } + if (ptr_info.duplicate_token) |duplicate| { + return gz.astgen.failTok(duplicate, "Extra pointer qualifier", .{}); + } const source_offset = gz.astgen.source_offset; const source_line = gz.astgen.source_line; diff --git a/lib/std/zig/Parse.zig b/lib/std/zig/Parse.zig index 6fc64faa8358000748947d1a047a98dc06e7d4dd..9a4f800851cd767e5c8c082257b42c8a138ca8c4 100644 --- a/lib/std/zig/Parse.zig +++ b/lib/std/zig/Parse.zig @@ -1691,8 +1691,8 @@ fn expectPrefixExpr(p: *Parse) Error!Node.Index { /// PrefixTypeOp /// <- QUESTIONMARK /// / KEYWORD_anyframe MINUSRARROW -/// / (ManyPtrTypeStart / SliceTypeStart) KEYWORD_allowzero? ByteAlign? AddrSpace? KEYWORD_const? KEYWORD_volatile? -/// / SinglePtrTypeStart KEYWORD_allowzero? BitAlign? AddrSpace? KEYWORD_const? KEYWORD_volatile? +/// / (ManyPtrTypeStart / SliceTypeStart) PtrMods +/// / SinglePtrTypeStart SinglePtrMods /// / ArrayTypeStart /// /// PrefixTypeOpPrefix @@ -1708,8 +1708,6 @@ fn expectPrefixExpr(p: *Parse) Error!Node.Index { /// ManyPtrTypeStart <- LBRACKET ASTERISK (LETTERC / COLON Expr)? RBRACKET /// /// ArrayTypeStart <- LBRACKET !ASTERISK Expr (COLON Expr)? RBRACKET -/// -/// BitAlign <- KEYWORD_align LPAREN Expr (COLON Expr COLON Expr)? RPAREN fn parseTypeExpr(p: *Parse) Error!?Node.Index { switch (p.tokenTag(p.tok_i)) { .question_mark => return try p.addNode(.{ @@ -3005,6 +3003,22 @@ const PtrModifiers = struct { bit_range_end: Node.OptionalIndex, }; +/// PtrMods +/// <- PtrMod* ByteAlign? PtrMod* AddrSpace? PtrMod* +/// / PtrMod* AddrSpace? PtrMod* ByteAlign? PtrMod* +/// +/// SinglePtrMods +/// <- PtrMod* BitAlign? PtrMod* AddrSpace? PtrMod* +/// / PtrMod* AddrSpace? PtrMod* BitAlign? PtrMod* +/// +/// PtrMod +/// <- KEYWORD_allowzero +/// / KEYWORD_const +/// / KEYWORD_volatile +/// +/// AddrSpace <- KEYWORD_addrspace LPAREN Expr RPAREN +/// ByteAlign <- KEYWORD_align LPAREN Expr RPAREN +/// BitAlign <- KEYWORD_align LPAREN Expr (COLON Expr COLON Expr)? RPAREN fn parsePtrModifiers(p: *Parse) !PtrModifiers { var result: PtrModifiers = .{ .align_node = .none, @@ -3012,9 +3026,6 @@ fn parsePtrModifiers(p: *Parse) !PtrModifiers { .bit_range_start = .none, .bit_range_end = .none, }; - var saw_const = false; - var saw_volatile = false; - var saw_allowzero = false; while (true) { switch (p.tokenTag(p.tok_i)) { .keyword_align => { @@ -3033,33 +3044,16 @@ fn parsePtrModifiers(p: *Parse) !PtrModifiers { _ = try p.expectToken(.r_paren); }, - .keyword_const => { - if (saw_const) { - try p.warn(.extra_const_qualifier); - } - p.tok_i += 1; - saw_const = true; - }, - .keyword_volatile => { - if (saw_volatile) { - try p.warn(.extra_volatile_qualifier); - } - p.tok_i += 1; - saw_volatile = true; - }, - .keyword_allowzero => { - if (saw_allowzero) { - try p.warn(.extra_allowzero_qualifier); - } - p.tok_i += 1; - saw_allowzero = true; - }, .keyword_addrspace => { if (result.addrspace_node != .none) { try p.warn(.extra_addrspace_qualifier); } result.addrspace_node = .fromOptional(try p.parseAddrSpace()); }, + .keyword_allowzero, + .keyword_const, + .keyword_volatile, + => p.tok_i += 1, else => return result, } } diff --git a/lib/std/zig/parser_fuzz.zig b/lib/std/zig/parser_fuzz.zig index 75b17d60a03e01816dc8947530d159c431570b7c..38d40fead34d0f3d899024dfbb2eb73c7fbfde1c 100644 --- a/lib/std/zig/parser_fuzz.zig +++ b/lib/std/zig/parser_fuzz.zig @@ -98,6 +98,11 @@ test "dot question" { try checkAgainstOracle("0. ?"); } +// Found using AFL++ +test "volatile const" { + try checkAgainstOracle("*volatile\nconst\n0"); +} + fn checkAgainstOracle(source: [:0]const u8) !void { var fba_buf: [1 << 18]u8 = undefined; var fba: std.heap.FixedBufferAllocator = .init(&fba_buf); diff --git a/lib/std/zig/parser_generated_oracle.zig b/lib/std/zig/parser_generated_oracle.zig index 264e19b649deed5706d540a8c0accdb631dfca63..19725cc742a54cb12159626caa985251b0938d1a 100644 --- a/lib/std/zig/parser_generated_oracle.zig +++ b/lib/std/zig/parser_generated_oracle.zig @@ -1751,15 +1751,83 @@ const Parser = struct { if (p.parseSliceTypeStart()) break :blk_2 true; p.i = pos_2; break :blk_2 false; - } and (p.parseKEYWORD_allowzero() or true) and (p.parseByteAlign() or true) and (p.parseAddrSpace() or true) and (p.parseKEYWORD_const() or true) and (p.parseKEYWORD_volatile() or true)) break :blk_0 true; + } and p.parsePtrMods()) break :blk_0 true; p.i = pos_0; - if (p.parseSinglePtrTypeStart() and (p.parseKEYWORD_allowzero() or true) and (p.parseBitAlign() or true) and (p.parseAddrSpace() or true) and (p.parseKEYWORD_const() or true) and (p.parseKEYWORD_volatile() or true)) break :blk_0 true; + if (p.parseSinglePtrTypeStart() and p.parseSinglePtrMods()) break :blk_0 true; p.i = pos_0; if (p.parseArrayTypeStart()) break :blk_0 true; p.i = pos_0; break :blk_0 false; }; } + pub fn parsePtrMods(p: *Parser) bool { + return blk_0: { + const pos_0 = p.i; + if (blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + } and (p.parseByteAlign() or true) and blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + } and (p.parseAddrSpace() or true) and blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + }) break :blk_0 true; + p.i = pos_0; + if (blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + } and (p.parseAddrSpace() or true) and blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + } and (p.parseByteAlign() or true) and blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + }) break :blk_0 true; + p.i = pos_0; + break :blk_0 false; + }; + } + pub fn parseSinglePtrMods(p: *Parser) bool { + return blk_0: { + const pos_0 = p.i; + if (blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + } and (p.parseBitAlign() or true) and blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + } and (p.parseAddrSpace() or true) and blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + }) break :blk_0 true; + p.i = pos_0; + if (blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + } and (p.parseAddrSpace() or true) and blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + } and (p.parseBitAlign() or true) and blk_1: { + while (p.parsePtrMod()) {} + break :blk_1 true; + }) break :blk_0 true; + p.i = pos_0; + break :blk_0 false; + }; + } + pub fn parsePtrMod(p: *Parser) bool { + return blk_0: { + const pos_0 = p.i; + if (p.parseKEYWORD_allowzero()) break :blk_0 true; + p.i = pos_0; + if (p.parseKEYWORD_const()) break :blk_0 true; + p.i = pos_0; + if (p.parseKEYWORD_volatile()) break :blk_0 true; + p.i = pos_0; + break :blk_0 false; + }; + } pub fn parsePrefixTypeOpPrefix(p: *Parser) bool { return blk_0: { const pos_0 = p.i; diff --git a/lib/std/zig/parser_test.zig b/lib/std/zig/parser_test.zig index 4e839c0d998e9728d3747ab233112d6e888d91b0..3f54d2217453eb547c43d6c0877cf9847a7aa821 100644 --- a/lib/std/zig/parser_test.zig +++ b/lib/std/zig/parser_test.zig @@ -6992,10 +6992,10 @@ test "recovery: non-associative operators" { test "recovery: extra qualifier" { try testError( - \\const a: *const const u8; + \\const a: *align(4) align(8) u8; \\test "" , &[_]Error{ - .extra_const_qualifier, + .extra_align_qualifier, .expected_block, }); } -- 2.54.0