authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2023-10-20 03:49:14-04:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2023-10-20 03:49:14-04:00
loga361f37b1c247a8a05383ceee48d0e2885f5bcd8
treeb33c046798d2329332f955a83375df93b7de2afd
parentdb18b562acd7d7dd3a35880aabee3b1c34fb043a
parent8ec04b567e3f37b82b870b6c595419905cbfcafd
signaturebadge-question-mark Signed by PGP key 4AEE18F83AFDEB23

Merge pull request #17608 from squeek502/resinator-fixes

resinator: Fix `INCLUDE` var handling and sync with upstream

9 files changed, 129 insertions(+), 80 deletions(-)

lib/std/Build/Step/Compile.zig+2
......@@ -244,6 +244,8 @@ pub const RcSourceFile = struct {
244244 file: LazyPath,
245245 /// Any option that rc.exe accepts will work here, with the exception of:
246246 /// - `/fo`: The output filename is set by the build system
247 /// - `/p`: Only running the preprocessor is not supported in this context
248 /// - `/:no-preprocess` (non-standard option): Not supported in this context
247249 /// - Any MUI-related option
248250 /// https://learn.microsoft.com/en-us/windows/win32/menurc/using-rc-the-rc-command-line-
249251 ///
src/Compilation.zig+12-2
......@@ -4766,11 +4766,21 @@ fn updateWin32Resource(comp: *Compilation, win32_resource: *Win32Resource, win32
47664766 };
47674767 defer options.deinit();
47684768
4769 // We never want to read the INCLUDE environment variable, so
4770 // unconditionally set `ignore_include_env_var` to true
4771 options.ignore_include_env_var = true;
4772
4773 if (options.preprocess != .yes) {
4774 return comp.failWin32Resource(win32_resource, "the '{s}' option is not supported in this context", .{switch (options.preprocess) {
4775 .no => "/:no-preprocess",
4776 .only => "/p",
4777 .yes => unreachable,
4778 }});
4779 }
4780
47694781 var argv = std.ArrayList([]const u8).init(comp.gpa);
47704782 defer argv.deinit();
47714783
4772 // TODO: support options.preprocess == .no and .only
4773 // alternatively, error if those options are used
47744784 try argv.appendSlice(&[_][]const u8{ self_exe_path, "clang" });
47754785
47764786 try resinator.preprocess.appendClangArgs(arena, &argv, options, .{
src/introspect.zig-2
......@@ -157,8 +157,6 @@ pub const EnvVar = enum {
157157 NO_COLOR,
158158 XDG_CACHE_HOME,
159159 HOME,
160 /// https://github.com/ziglang/zig/issues/17585
161 INCLUDE,
162160
163161 pub fn isSet(comptime ev: EnvVar) bool {
164162 return std.process.hasEnvVarConstant(@tagName(ev));
src/resinator/compile.zig+25-8
......@@ -28,7 +28,6 @@ const windows1252 = @import("windows1252.zig");
2828const lang = @import("lang.zig");
2929const code_pages = @import("code_pages.zig");
3030const errors = @import("errors.zig");
31const introspect = @import("../introspect.zig");
3231
3332pub const CompileOptions = struct {
3433 cwd: std.fs.Dir,
......@@ -89,10 +88,23 @@ pub fn compile(allocator: Allocator, source: []const u8, writer: anytype, option
8988 }
9089 }
9190 // Re-open the passed in cwd since we want to be able to close it (std.fs.cwd() shouldn't be closed)
92 // `catch unreachable` since `options.cwd` is expected to be a valid dir handle, so opening
93 // a new handle to it should be fine as well.
94 // TODO: Maybe catch and return an error instead
95 const cwd_dir = options.cwd.openDir(".", .{}) catch @panic("unable to open dir");
91 const cwd_dir = options.cwd.openDir(".", .{}) catch |err| {
92 try options.diagnostics.append(.{
93 .err = .failed_to_open_cwd,
94 .token = .{
95 .id = .invalid,
96 .start = 0,
97 .end = 0,
98 .line_number = 1,
99 },
100 .print_source_line = false,
101 .extra = .{ .file_open_error = .{
102 .err = ErrorDetails.FileOpenError.enumFromError(err),
103 .filename_string_index = undefined,
104 } },
105 });
106 return error.CompileError;
107 };
96108 try search_dirs.append(.{ .dir = cwd_dir, .path = null });
97109 for (options.extra_include_paths) |extra_include_path| {
98110 var dir = openSearchPathDir(options.cwd, extra_include_path) catch {
......@@ -111,11 +123,16 @@ pub fn compile(allocator: Allocator, source: []const u8, writer: anytype, option
111123 try search_dirs.append(.{ .dir = dir, .path = try allocator.dupe(u8, system_include_path) });
112124 }
113125 if (!options.ignore_include_env_var) {
114 const INCLUDE = (introspect.EnvVar.INCLUDE.get(allocator) catch @panic("OOM")) orelse "";
126 const INCLUDE = std.process.getEnvVarOwned(allocator, "INCLUDE") catch "";
115127 defer allocator.free(INCLUDE);
116128
117 // TODO: Should this be platform-specific? How does windres/llvm-rc handle this (if at all)?
118 var it = std.mem.tokenize(u8, INCLUDE, ";");
129 // The only precedence here is llvm-rc which also uses the platform-specific
130 // delimiter. There's no precedence set by `rc.exe` since it's Windows-only.
131 const delimiter = switch (builtin.os.tag) {
132 .windows => ';',
133 else => ':',
134 };
135 var it = std.mem.tokenizeScalar(u8, INCLUDE, delimiter);
119136 while (it.next()) |search_path| {
120137 var dir = openSearchPathDir(options.cwd, search_path) catch continue;
121138 errdefer dir.close();
src/resinator/errors.zig+22-9
......@@ -395,6 +395,10 @@ pub const ErrorDetails = struct {
395395 // General (used in various places)
396396 /// `number` is populated and contains the value that the ordinal would have in the Win32 RC compiler implementation
397397 win32_non_ascii_ordinal,
398
399 // Initialization
400 /// `file_open_error` is populated, but `filename_string_index` is not
401 failed_to_open_cwd,
398402 };
399403
400404 pub fn render(self: ErrorDetails, writer: anytype, source: []const u8, strings: []const []const u8) !void {
......@@ -766,6 +770,9 @@ pub const ErrorDetails = struct {
766770 .note => return writer.print("the Win32 RC compiler would accept this as an ordinal but its value would be {}", .{self.extra.number}),
767771 .hint => return,
768772 },
773 .failed_to_open_cwd => {
774 try writer.print("failed to open CWD for compilation: {s}", .{@tagName(self.extra.file_open_error.err)});
775 },
769776 }
770777 }
771778
......@@ -804,7 +811,8 @@ pub const ErrorDetails = struct {
804811 .point_offset = self.token.start - source_line_start,
805812 .after_len = after: {
806813 const end = @min(source_line_end, if (self.token_span_end) |span_end| span_end.end else self.token.end);
807 if (end == self.token.start) break :after 0;
814 // end may be less than start when pointing to EOF
815 if (end <= self.token.start) break :after 0;
808816 break :after end - self.token.start - 1;
809817 },
810818 },
......@@ -816,13 +824,18 @@ pub fn renderErrorMessage(allocator: std.mem.Allocator, writer: anytype, tty_con
816824 if (err_details.type == .hint) return;
817825
818826 const source_line_start = err_details.token.getLineStart(source);
819 const column = err_details.token.calculateColumn(source, 1, source_line_start);
820
821 // var counting_writer_container = std.io.countingWriter(writer);
822 // const counting_writer = counting_writer_container.writer();
823
824 const corresponding_span: ?SourceMappings.SourceSpan = if (source_mappings) |mappings| mappings.get(err_details.token.line_number) else null;
825 const corresponding_file: ?[]const u8 = if (source_mappings) |mappings| mappings.files.get(corresponding_span.?.filename_offset) else null;
827 // Treat tab stops as 1 column wide for error display purposes,
828 // and add one to get a 1-based column
829 const column = err_details.token.calculateColumn(source, 1, source_line_start) + 1;
830
831 const corresponding_span: ?SourceMappings.SourceSpan = if (source_mappings != null and source_mappings.?.has(err_details.token.line_number))
832 source_mappings.?.get(err_details.token.line_number)
833 else
834 null;
835 const corresponding_file: ?[]const u8 = if (source_mappings != null and corresponding_span != null)
836 source_mappings.?.files.get(corresponding_span.?.filename_offset)
837 else
838 null;
826839
827840 const err_line = if (corresponding_span) |span| span.start_line else err_details.token.line_number;
828841
......@@ -897,7 +910,7 @@ pub fn renderErrorMessage(allocator: std.mem.Allocator, writer: anytype, tty_con
897910 try writer.writeByte('\n');
898911 try tty_config.setColor(writer, .reset);
899912
900 if (source_mappings) |_| {
913 if (corresponding_span != null and corresponding_file != null) {
901914 var corresponding_lines = try CorrespondingLines.init(allocator, cwd, err_details, source_line_for_display_buf.items, corresponding_span.?, corresponding_file.?);
902915 defer corresponding_lines.deinit(allocator);
903916
src/resinator/lex.zig+48-54
......@@ -6,7 +6,7 @@
66
77const std = @import("std");
88const ErrorDetails = @import("errors.zig").ErrorDetails;
9const columnsUntilTabStop = @import("literals.zig").columnsUntilTabStop;
9const columnWidth = @import("literals.zig").columnWidth;
1010const code_pages = @import("code_pages.zig");
1111const CodePage = code_pages.CodePage;
1212const SourceMappings = @import("source_mapping.zig").SourceMappings;
......@@ -69,17 +69,14 @@ pub const Token = struct {
6969 };
7070 }
7171
72 /// Returns 0-based column
7273 pub fn calculateColumn(token: Token, source: []const u8, tab_columns: usize, maybe_line_start: ?usize) usize {
7374 const line_start = maybe_line_start orelse token.getLineStart(source);
7475
7576 var i: usize = line_start;
7677 var column: usize = 0;
7778 while (i < token.start) : (i += 1) {
78 const c = source[i];
79 switch (c) {
80 '\t' => column += columnsUntilTabStop(column, tab_columns),
81 else => column += 1,
82 }
79 column += columnWidth(column, source[i], tab_columns);
8380 }
8481 return column;
8582 }
......@@ -109,6 +106,7 @@ pub const Token = struct {
109106 const line_start = maybe_line_start orelse token.getLineStart(source);
110107
111108 var line_end = line_start + 1;
109 if (line_end >= source.len or source[line_end] == '\n') return source[line_start..line_start];
112110 while (line_end < source.len and source[line_end] != '\n') : (line_end += 1) {}
113111 while (line_end > 0 and source[line_end - 1] == '\r') : (line_end -= 1) {}
114112
......@@ -404,6 +402,9 @@ pub const Lexer = struct {
404402 // TODO: Understand this more, bring it more in line with how the Win32 limits work.
405403 // Alternatively, do something that makes more sense but may be more permissive.
406404 var string_literal_length: usize = 0;
405 // Keeping track of the string literal column prevents pathological edge cases when
406 // there are tons of tab stop characters within a string literal.
407 var string_literal_column: usize = 0;
407408 var string_literal_collapsing_whitespace: bool = false;
408409 var still_could_have_exponent: bool = true;
409410 var exponent_index: ?usize = null;
......@@ -471,6 +472,14 @@ pub const Lexer = struct {
471472 self.at_start_of_line = false;
472473 string_literal_collapsing_whitespace = false;
473474 string_literal_length = 0;
475
476 var dummy_token = Token{
477 .start = self.index,
478 .end = self.index,
479 .line_number = self.line_handler.line_number,
480 .id = .invalid,
481 };
482 string_literal_column = dummy_token.calculateColumn(self.buffer, 8, null);
474483 },
475484 '+', '&', '|' => {
476485 self.index += 1;
......@@ -618,6 +627,14 @@ pub const Lexer = struct {
618627 state = .quoted_wide_string;
619628 string_literal_collapsing_whitespace = false;
620629 string_literal_length = 0;
630
631 var dummy_token = Token{
632 .start = self.index,
633 .end = self.index,
634 .line_number = self.line_handler.line_number,
635 .id = .invalid,
636 };
637 string_literal_column = dummy_token.calculateColumn(self.buffer, 8, null);
621638 },
622639 else => {
623640 state = .literal;
......@@ -695,18 +712,23 @@ pub const Lexer = struct {
695712 },
696713 .quoted_ascii_string, .quoted_wide_string => switch (c) {
697714 '"' => {
715 string_literal_column += 1;
698716 state = if (state == .quoted_ascii_string) .quoted_ascii_string_maybe_end else .quoted_wide_string_maybe_end;
699717 },
700718 '\\' => {
719 string_literal_length += 1;
720 string_literal_column += 1;
701721 state = if (state == .quoted_ascii_string) .quoted_ascii_string_escape else .quoted_wide_string_escape;
702722 },
703723 '\r' => {
724 string_literal_column = 0;
704725 // \r doesn't count towards string literal length
705726
706727 // Increment line number but don't affect the result token's line number
707728 _ = self.incrementLineNumber();
708729 },
709730 '\n' => {
731 string_literal_column = 0;
710732 // first \n expands to <space><\n>
711733 if (!string_literal_collapsing_whitespace) {
712734 string_literal_length += 2;
......@@ -720,33 +742,17 @@ pub const Lexer = struct {
720742 // only \t, space, Vertical Tab, and Form Feed count as whitespace when collapsing
721743 '\t', ' ', '\x0b', '\x0c' => {
722744 if (!string_literal_collapsing_whitespace) {
723 if (c == '\t') {
724 // Literal tab characters are counted as the number of space characters
725 // needed to reach the next 8-column tab stop.
726 //
727 // This implemention is ineffecient but hopefully it's enough of an
728 // edge case that it doesn't matter too much. Literal tab characters in
729 // string literals being replaced by a variable number of spaces depending
730 // on which column the tab character is located in the source .rc file seems
731 // like it has extremely limited use-cases, so it seems unlikely that it's used
732 // in real .rc files.
733 var dummy_token = Token{
734 .start = self.index,
735 .end = self.index,
736 .line_number = self.line_handler.line_number,
737 .id = .invalid,
738 };
739 dummy_token.start = self.index;
740 const current_column = dummy_token.calculateColumn(self.buffer, 8, null);
741 string_literal_length += columnsUntilTabStop(current_column, 8);
742 } else {
743 string_literal_length += 1;
744 }
745 // Literal tab characters are counted as the number of space characters
746 // needed to reach the next 8-column tab stop.
747 const width = columnWidth(string_literal_column, @intCast(c), 8);
748 string_literal_length += width;
749 string_literal_column += width;
745750 }
746751 },
747752 else => {
748753 string_literal_collapsing_whitespace = false;
749754 string_literal_length += 1;
755 string_literal_column += 1;
750756 },
751757 },
752758 .quoted_ascii_string_escape, .quoted_wide_string_escape => switch (c) {
......@@ -760,14 +766,19 @@ pub const Lexer = struct {
760766 return error.FoundCStyleEscapedQuote;
761767 },
762768 else => {
769 string_literal_length += 1;
770 string_literal_column += 1;
763771 state = if (state == .quoted_ascii_string_escape) .quoted_ascii_string else .quoted_wide_string;
764772 },
765773 },
766774 .quoted_ascii_string_maybe_end, .quoted_wide_string_maybe_end => switch (c) {
767775 '"' => {
768776 state = if (state == .quoted_ascii_string_maybe_end) .quoted_ascii_string else .quoted_wide_string;
769 // Escaped quotes only count as 1 char for string literal length checks,
770 // so we don't increment string_literal_length here.
777 // Escaped quotes count as 1 char for string literal length checks.
778 // Since we did not increment on the first " (because it could have been
779 // the end of the quoted string), we increment here
780 string_literal_length += 1;
781 string_literal_column += 1;
771782 },
772783 else => {
773784 result.id = if (state == .quoted_ascii_string_maybe_end) .quoted_ascii_string else .quoted_wide_string;
......@@ -807,6 +818,8 @@ pub const Lexer = struct {
807818 }
808819 }
809820
821 result.end = self.index;
822
810823 if (result.id == .quoted_ascii_string or result.id == .quoted_wide_string) {
811824 if (string_literal_length > self.max_string_literal_codepoints) {
812825 self.error_context_token = result;
......@@ -814,7 +827,6 @@ pub const Lexer = struct {
814827 }
815828 }
816829
817 result.end = self.index;
818830 return result;
819831 }
820832
......@@ -877,6 +889,7 @@ pub const Lexer = struct {
877889 .end = end,
878890 .line_number = self.line_handler.line_number,
879891 };
892 errdefer self.error_context_token = token;
880893 const full_command = self.buffer[start..end];
881894 var command = full_command;
882895
......@@ -901,7 +914,6 @@ pub const Lexer = struct {
901914 }
902915
903916 if (command.len == 0 or command[0] != '(') {
904 self.error_context_token = token;
905917 return error.CodePagePragmaMissingLeftParen;
906918 }
907919 command = command[1..];
......@@ -917,7 +929,6 @@ pub const Lexer = struct {
917929 }
918930
919931 if (num_str.len == 0) {
920 self.error_context_token = token;
921932 return error.CodePagePragmaNotInteger;
922933 }
923934
......@@ -926,7 +937,6 @@ pub const Lexer = struct {
926937 }
927938
928939 if (command.len == 0 or command[0] != ')') {
929 self.error_context_token = token;
930940 return error.CodePagePragmaMissingRightParen;
931941 }
932942
......@@ -943,41 +953,26 @@ pub const Lexer = struct {
943953 //
944954 // Instead of that, we just have a separate error specifically for overflow.
945955 const num = parseCodePageNum(num_str) catch |err| switch (err) {
946 error.InvalidCharacter => {
947 self.error_context_token = token;
948 return error.CodePagePragmaNotInteger;
949 },
950 error.Overflow => {
951 self.error_context_token = token;
952 return error.CodePagePragmaOverflow;
953 },
956 error.InvalidCharacter => return error.CodePagePragmaNotInteger,
957 error.Overflow => return error.CodePagePragmaOverflow,
954958 };
955959
956960 // Anything that starts with 0 but does not resolve to 0 is treated as invalid, e.g. 01252
957961 if (num_str[0] == '0' and num != 0) {
958 self.error_context_token = token;
959962 return error.CodePagePragmaInvalidCodePage;
960963 }
961964 // Anything that resolves to 0 is treated as 'not an integer' by the Win32 implementation.
962965 else if (num == 0) {
963 self.error_context_token = token;
964966 return error.CodePagePragmaNotInteger;
965967 }
966968 // Anything above u16 max is not going to be found since our CodePage enum is backed by a u16.
967969 if (num > std.math.maxInt(u16)) {
968 self.error_context_token = token;
969970 return error.CodePagePragmaInvalidCodePage;
970971 }
971972
972973 break :code_page code_pages.CodePage.getByIdentifierEnsureSupported(@intCast(num)) catch |err| switch (err) {
973 error.InvalidCodePage => {
974 self.error_context_token = token;
975 return error.CodePagePragmaInvalidCodePage;
976 },
977 error.UnsupportedCodePage => {
978 self.error_context_token = token;
979 return error.CodePagePragmaUnsupportedCodePage;
980 },
974 error.InvalidCodePage => return error.CodePagePragmaInvalidCodePage,
975 error.UnsupportedCodePage => return error.CodePagePragmaUnsupportedCodePage,
981976 };
982977 };
983978
......@@ -990,7 +985,6 @@ pub const Lexer = struct {
990985 // to still be able to work correctly after this error is returned.
991986 if (self.source_mappings) |source_mappings| {
992987 if (!source_mappings.isRootFile(token.line_number)) {
993 self.error_context_token = token;
994988 return error.CodePagePragmaInIncludedFile;
995989 }
996990 }
src/resinator/literals.zig+7
......@@ -775,6 +775,13 @@ pub fn columnsUntilTabStop(column: usize, tab_columns: usize) usize {
775775 return tab_columns - (column % tab_columns);
776776}
777777
778pub fn columnWidth(cur_column: usize, c: u8, tab_columns: usize) usize {
779 return switch (c) {
780 '\t' => columnsUntilTabStop(cur_column, tab_columns),
781 else => 1,
782 };
783}
784
778785pub const Number = struct {
779786 value: u32,
780787 is_long: bool = false,
src/resinator/preprocess.zig+9-4
......@@ -1,7 +1,7 @@
11const std = @import("std");
2const builtin = @import("builtin");
23const Allocator = std.mem.Allocator;
34const cli = @import("cli.zig");
4const introspect = @import("../introspect.zig");
55
66pub const IncludeArgs = struct {
77 clang_target: ?[]const u8 = null,
......@@ -68,10 +68,15 @@ pub fn appendClangArgs(arena: Allocator, argv: *std.ArrayList([]const u8), optio
6868 }
6969
7070 if (!options.ignore_include_env_var) {
71 const INCLUDE = (introspect.EnvVar.INCLUDE.get(arena) catch @panic("OOM")) orelse "";
71 const INCLUDE = std.process.getEnvVarOwned(arena, "INCLUDE") catch "";
7272
73 // TODO: Should this be platform-specific? How does windres/llvm-rc handle this (if at all)?
74 var it = std.mem.tokenize(u8, INCLUDE, ";");
73 // The only precedence here is llvm-rc which also uses the platform-specific
74 // delimiter. There's no precedence set by `rc.exe` since it's Windows-only.
75 const delimiter = switch (builtin.os.tag) {
76 .windows => ';',
77 else => ':',
78 };
79 var it = std.mem.tokenizeScalar(u8, INCLUDE, delimiter);
7580 while (it.next()) |include_path| {
7681 try argv.append("-isystem");
7782 try argv.append(include_path);
src/resinator/source_mapping.zig+4-1
......@@ -240,6 +240,9 @@ pub fn handleLineCommand(allocator: Allocator, line_command: []const u8, current
240240 };
241241 defer allocator.free(filename);
242242
243 // \x00 bytes in the filename is incompatible with how StringTable works
244 if (std.mem.indexOfScalar(u8, filename, '\x00') != null) return;
245
243246 current_mapping.line_num = linenum;
244247 current_mapping.filename.clearRetainingCapacity();
245248 try current_mapping.filename.appendSlice(allocator, filename);
......@@ -441,7 +444,7 @@ pub const SourceMappings = struct {
441444 ptr.* = span;
442445 }
443446
444 pub fn has(self: *SourceMappings, line_num: usize) bool {
447 pub fn has(self: SourceMappings, line_num: usize) bool {
445448 return self.mapping.items.len >= line_num;
446449 }
447450