diff --git a/src/compile.zig b/src/compile.zig index 911290f..6c9d301 100644 --- a/src/compile.zig +++ b/src/compile.zig @@ -1395,8 +1395,6 @@ pub const Compiler = struct { menu.deinit(self.allocator); } } - var skipped_menu_or_classes = std.ArrayList(*Node.SimpleStatement).init(self.allocator); - defer skipped_menu_or_classes.deinit(); var last_menu: *Node.SimpleStatement = undefined; var last_class: *Node.SimpleStatement = undefined; var last_menu_would_be_forced_ordinal = false; @@ -1426,9 +1424,6 @@ pub const Compiler = struct { }, .class => { const is_duplicate = optional_statement_values.class != null; - if (is_duplicate) { - try skipped_menu_or_classes.append(last_class); - } const forced_ordinal = is_duplicate and optional_statement_values.class.? == .ordinal; // In the Win32 RC compiler, if any CLASS values that are interpreted as // an ordinal exist, it affects all future CLASS statements and forces @@ -1456,9 +1451,6 @@ pub const Compiler = struct { }, .menu => { const is_duplicate = optional_statement_values.menu != null; - if (is_duplicate) { - try skipped_menu_or_classes.append(last_menu); - } const forced_ordinal = is_duplicate and optional_statement_values.menu.? == .ordinal; // In the Win32 RC compiler, if any MENU values that are interpreted as // an ordinal exist, it affects all future MENU statements and forces @@ -1542,22 +1534,6 @@ pub const Compiler = struct { } } - for (skipped_menu_or_classes.items) |simple_statement| { - const statement_identifier = simple_statement.identifier; - const statement_type = rc.OptionalStatements.dialog_map.get(statement_identifier.slice(self.source)) orelse continue; - try self.addErrorDetails(.{ - .err = .duplicate_menu_or_class_skipped, - .type = .warning, - .token = simple_statement.identifier, - .token_span_start = simple_statement.base.getFirstToken(), - .token_span_end = simple_statement.base.getLastToken(), - .extra = .{ .menu_or_class = switch (statement_type) { - .menu => .menu, - .class => .class, - else => unreachable, - } }, - }); - } // The Win32 RC compiler miscompiles the value in the following scenario: // Multiple CLASS parameters are specified and any of them are treated as a number, then // the last CLASS is always treated as a number no matter what diff --git a/src/errors.zig b/src/errors.zig index a713bdd..1e62abc 100644 --- a/src/errors.zig +++ b/src/errors.zig @@ -379,8 +379,7 @@ pub const ErrorDetails = struct { rc_would_miscompile_dialog_menu_or_class_id_forced_ordinal, rc_would_miscompile_dialog_menu_id_starts_with_digit, dialog_menu_id_was_uppercased, - /// `menu_or_class` is populated and contains the type of the parameter statement - duplicate_menu_or_class_skipped, + duplicate_optional_statement_skipped, invalid_digit_character_in_ordinal, // Literals @@ -740,11 +739,8 @@ pub const ErrorDetails = struct { .hint => return, }, .dialog_menu_id_was_uppercased => return, - .duplicate_menu_or_class_skipped => { - return writer.print("this {s} was ignored; when multiple {s} statements are specified, only the last takes precedence", .{ - @tagName(self.extra.menu_or_class), - @tagName(self.extra.menu_or_class), - }); + .duplicate_optional_statement_skipped => { + return writer.writeAll("this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence"); }, .invalid_digit_character_in_ordinal => { return writer.writeAll("non-ASCII digit characters are not allowed in ordinal (number) values"); diff --git a/src/parse.zig b/src/parse.zig index fff1e45..28553fd 100644 --- a/src/parse.zig +++ b/src/parse.zig @@ -132,6 +132,11 @@ pub const Parser = struct { /// The returned slice is allocated by the parser's arena fn parseOptionalStatements(self: *Self, resource: Resource) ![]*Node { var optional_statements = std.ArrayListUnmanaged(*Node){}; + + const num_statement_types = @typeInfo(rc.OptionalStatements).Enum.fields.len; + var statement_type_has_duplicates = [_]bool{false} ** num_statement_types; + var last_statement_per_type = [_]?*Node{null} ** num_statement_types; + while (true) { const lookahead_token = try self.lookaheadToken(.normal); if (lookahead_token.id != .literal) break; @@ -141,6 +146,12 @@ pub const Parser = struct { else => break, }; self.nextToken(.normal) catch unreachable; + + const type_i = @intFromEnum(optional_statement_type); + if (last_statement_per_type[type_i] != null) { + statement_type_has_duplicates[type_i] = true; + } + switch (optional_statement_type) { .language => { const language = try self.parseLanguageStatement(); @@ -272,7 +283,42 @@ pub const Parser = struct { try optional_statements.append(self.state.arena, &node.base); }, } + + last_statement_per_type[type_i] = optional_statements.items[optional_statements.items.len - 1]; + } + + for (optional_statements.items) |optional_statement| { + const type_i = type_i: { + switch (optional_statement.id) { + .simple_statement => { + const simple_statement: *Node.SimpleStatement = @alignCast(@fieldParentPtr("base", optional_statement)); + const statement_identifier = simple_statement.identifier; + const slice = statement_identifier.slice(self.lexer.buffer); + const optional_statement_type = rc.OptionalStatements.map.get(slice) orelse + rc.OptionalStatements.dialog_map.get(slice).?; + break :type_i @intFromEnum(optional_statement_type); + }, + .font_statement => { + break :type_i @intFromEnum(rc.OptionalStatements.font); + }, + .language_statement => { + break :type_i @intFromEnum(rc.OptionalStatements.language); + }, + else => unreachable, + } + }; + if (!statement_type_has_duplicates[type_i]) continue; + if (optional_statement == last_statement_per_type[type_i].?) continue; + + try self.addErrorDetails(.{ + .err = .duplicate_optional_statement_skipped, + .type = .warning, + .token = optional_statement.getFirstToken(), + .token_span_start = optional_statement.getFirstToken(), + .token_span_end = optional_statement.getLastToken(), + }); } + return optional_statements.toOwnedSlice(self.state.arena); } diff --git a/test/compile.zig b/test/compile.zig index 027395a..8c17d6c 100644 --- a/test/compile.zig +++ b/test/compile.zig @@ -683,8 +683,8 @@ test "dialog, dialogex resource" { ); try testCompileErrorDetailsWithDir( &.{ - .{ .type = .warning, .str = "this class was ignored; when multiple class statements are specified, only the last takes precedence" }, - .{ .type = .warning, .str = "this class was ignored; when multiple class statements are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, .{ .type = .warning, .str = "this class would be miscompiled by the Win32 RC compiler" }, .{ .type = .note, .str = "the Win32 RC compiler would evaluate it as the ordinal/number value 62790" }, .{ .type = .note, .str = "to avoid the potential miscompilation, only specify one class per dialog resource" }, diff --git a/test/parse.zig b/test/parse.zig index 0641f1a..0049a41 100644 --- a/test/parse.zig +++ b/test/parse.zig @@ -2178,6 +2178,46 @@ test "language with L suffixed part" { ); } +test "duplicate optional statements" { + try testParseErrorDetails( + &.{ + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + .{ .type = .warning, .str = "this statement was ignored; when multiple statements of the same type are specified, only the last takes precedence" }, + }, + \\1 DIALOGEX 1, 2, 3, 4 + \\ CHARACTERISTICS 1 + \\ LANGUAGE 0xFF, 0xFF + \\ LANGUAGE 0x09, 0x01 + \\ CHARACTERISTICS 999 + \\ CHARACTERISTICS 0x1234 + \\ VERSION 999 + \\ VERSION 1 + \\ MENU 1 + \\ MENU 2 + \\ CLASS "foo" + \\ CLASS "bar" + \\ CAPTION "foo" + \\ CAPTION "bar" + \\ FONT 1, "foo" + \\ FONT 2, "bar" + \\ STYLE 1 + \\ STYLE 2 + \\ EXSTYLE 1 + \\ EXSTYLE 2 + \\{} + , + null, + ); +} + fn testParse(source: []const u8, expected_ast_dump: []const u8) !void { const allocator = std.testing.allocator; var diagnostics = resinator.errors.Diagnostics.init(allocator);