From e730172e47749db8a9d3be5949231bde95343b39 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 7 Apr 2021 22:02:45 -0700 Subject: [PATCH] stage2: fix switch validation of handling all enum values There were several problems, all fixed: * AstGen was storing field names as references to the original source code bytes. However, that data would be destroyed when the source file is updated. Now, it correctly stores the field names in the Decl arena for the enum. The same fix applies to error set field names. * Sema was missing a memset inside `analyzeSwitch`, leaving the "seen enum fields" array with undefined memory. Now that they are all properly set to null, the validation works. * Moved the "enum declared here" note to the end. It looked weird interrupting the notes for which enum values were missing. --- src/Module.zig | 21 ++++++++++++++++----- src/Sema.zig | 16 +++++++++------- test/stage2/cbe.zig | 15 +++++++++++++++ 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/Module.zig b/src/Module.zig index ccf6996cae5b4033c97df298a4748c21da18e35f..933917d9484096d70710f2461170068da9b897fe 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -4582,28 +4582,39 @@ pub fn optimizeMode(mod: Module) std.builtin.Mode { /// Otherwise, returns a reference to the source code bytes directly. /// See also `appendIdentStr` and `parseStrLit`. pub fn identifierTokenString(mod: *Module, scope: *Scope, token: ast.TokenIndex) InnerError![]const u8 { - return mod.identifierTokenStringTreeArena(scope, token, scope.tree(), scope.arena()); + const tree = scope.tree(); + const token_tags = tree.tokens.items(.tag); + assert(token_tags[token] == .identifier); + const ident_name = tree.tokenSlice(token); + if (!mem.startsWith(u8, ident_name, "@")) { + return ident_name; + } + var buf: ArrayListUnmanaged(u8) = .{}; + defer buf.deinit(mod.gpa); + try parseStrLit(mod, scope, token, &buf, ident_name, 1); + const duped = try scope.arena().dupe(u8, buf.items); + return duped; } /// `scope` is only used for error reporting. +/// The string is stored in `arena` regardless of whether it uses @"" syntax. pub fn identifierTokenStringTreeArena( mod: *Module, scope: *Scope, token: ast.TokenIndex, tree: *const ast.Tree, arena: *Allocator, -) InnerError![]const u8 { +) InnerError![]u8 { const token_tags = tree.tokens.items(.tag); assert(token_tags[token] == .identifier); const ident_name = tree.tokenSlice(token); if (!mem.startsWith(u8, ident_name, "@")) { - return ident_name; + return arena.dupe(u8, ident_name); } var buf: ArrayListUnmanaged(u8) = .{}; defer buf.deinit(mod.gpa); try parseStrLit(mod, scope, token, &buf, ident_name, 1); - const duped = try arena.dupe(u8, buf.items); - return duped; + return arena.dupe(u8, buf.items); } /// Given an identifier token, obtain the string for it (possibly parsing as a string diff --git a/src/Sema.zig b/src/Sema.zig index 134e56994822fa569c1e62c2ddf564ddb8fe7817..a0be08ed715600f7fd879fe46c36dd9960a54caf 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -2789,6 +2789,8 @@ fn analyzeSwitch( var seen_fields = try gpa.alloc(?AstGen.SwitchProngSrc, operand.ty.enumFieldCount()); defer gpa.free(seen_fields); + mem.set(?AstGen.SwitchProngSrc, seen_fields, null); + var extra_index: usize = special.end; { var scalar_i: u32 = 0; @@ -2849,12 +2851,6 @@ fn analyzeSwitch( .{}, ); errdefer msg.destroy(sema.gpa); - try mod.errNoteNonLazy( - operand.ty.declSrcLoc(), - msg, - "enum '{}' declared here", - .{operand.ty}, - ); for (seen_fields) |seen_src, i| { if (seen_src != null) continue; @@ -2865,10 +2861,16 @@ fn analyzeSwitch( &block.base, src, msg, - "unhandled enumeration value: '{s}", + "unhandled enumeration value: '{s}'", .{field_name}, ); } + try mod.errNoteNonLazy( + operand.ty.declSrcLoc(), + msg, + "enum '{}' declared here", + .{operand.ty}, + ); break :msg msg; }; return mod.failWithOwnedErrorMsg(&block.base, msg); diff --git a/test/stage2/cbe.zig b/test/stage2/cbe.zig index b0386f6246133b0e8d23ff8b66a8d576f73ce55d..3cf95ab6d6e649d3a2942ee1698490f986474ae4 100644 --- a/test/stage2/cbe.zig +++ b/test/stage2/cbe.zig @@ -702,6 +702,21 @@ pub fn addCases(ctx: *TestContext) !void { ":3:15: error: enum 'E' has no tag with value 3", ":1:11: note: enum declared here", }); + + case.addError( + \\const E = enum { a, b, c }; + \\export fn foo() void { + \\ var x: E = .a; + \\ switch (x) { + \\ .a => {}, + \\ .c => {}, + \\ } + \\} + , &.{ + ":4:5: error: switch must handle all possibilities", + ":4:5: note: unhandled enumeration value: 'b'", + ":1:11: note: enum 'E' declared here", + }); } ctx.c("empty start function", linux_x64, -- 2.54.0