| author |  Tom Read Cutting <readcuttingt@gmail.com> 2023-02-19 12:14:03+00:00 |
| committer |  GitHub <noreply@github.com> 2023-02-19 14:14:03+02:00 |
| log | 346ec15c5005e523c2a1d4b967ee7a4e5d1e9775 |
| tree | 16f8b1bc34b30421f40c7d3b2aae5a770fe732b4 |
| parent | 281d4c0ff6f95de0090f3621b4bdb651cd3d0330 |
| signature |  Signed by PGP key 4AEE18F83AFDEB23 |
Correctly handle carriage return characters according to the spec (#12661)
* Scan from line start when finding tag in tokenizer
This resolves a crash that can occur for invalid bytes like carriage
returns that are valid characters when not parsed from within literals.
There are potentially other edge cases this could resolve as well, as
the calling code for this function didn't account for any potential
'pending_invalid_tokens' that could be queued up by the tokenizer from
within another state.
* Fix carriage return crash in multiline string
Follow the guidance of #38:
> However CR directly before NL is interpreted as only a newline and not part of the multiline string. zig fmt will delete the CR.
Zig fmt already had code for deleting carriage returns, but would still
crash - now it no longer does so. Carriage returns encountered before
line-feeds are now appropriately removed on program compilation as well.
* Only accept carriage returns before line feeds
Previous commit was much less strict about this, this more closely
matches the desired spec of only allow CR characters in a CRLF pair, but
not otherwise.
* Fix CR being rejected when used as whitespace
Missed this comment from ziglang/zig-spec#83:
> CR used as whitespace, whether directly preceding NL or stray, is still unambiguously whitespace. It is accepted by the grammar and replaced by the canonical whitespace by zig fmt.
* Add tests for carriage return handling6 files changed, 71 insertions(+), 9 deletions(-)
lib- std
- src
doc/langref.html.in+2-1
| ... | @@ -11551,7 +11551,8 @@ fn readU32Be() u32 {} | ... | @@ -11551,7 +11551,8 @@ fn readU32Be() u32 {} |
| 11551 | </p> | 11551 | </p> |
| 11552 | <p> | 11552 | <p> |
| 11553 | Each LF may be immediately preceded by a single CR (byte value 0x0d, code point U+000d, {#syntax#}'\r'{#endsyntax#}) | 11553 | Each LF may be immediately preceded by a single CR (byte value 0x0d, code point U+000d, {#syntax#}'\r'{#endsyntax#}) |
| 11554 | to form a Windows style line ending, but this is discouraged. | 11554 | to form a Windows style line ending, but this is discouraged. Note that in mulitline strings, CRLF sequences will |
| 11555 | be encoded as LF when compiled into a zig program. | ||
| 11555 | A CR in any other context is not allowed. | 11556 | A CR in any other context is not allowed. |
| 11556 | </p> | 11557 | </p> |
| 11557 | <p> | 11558 | <p> |
lib/std/zig/Ast.zig+1-1
| ... | @@ -171,7 +171,7 @@ pub fn tokenSlice(tree: Ast, token_index: TokenIndex) []const u8 { | ... | @@ -171,7 +171,7 @@ pub fn tokenSlice(tree: Ast, token_index: TokenIndex) []const u8 { |
| 171 | .index = token_starts[token_index], | 171 | .index = token_starts[token_index], |
| 172 | .pending_invalid_token = null, | 172 | .pending_invalid_token = null, |
| 173 | }; | 173 | }; |
| 174 | const token = tokenizer.next(); | 174 | const token = tokenizer.findTagAtCurrentIndex(token_tag); |
| 175 | assert(token.tag == token_tag); | 175 | assert(token.tag == token_tag); |
| 176 | return tree.source[token.loc.start..token.loc.end]; | 176 | return tree.source[token.loc.start..token.loc.end]; |
| 177 | } | 177 | } |
lib/std/zig/tokenizer.zig+45-5
| ... | @@ -406,6 +406,38 @@ pub const Tokenizer = struct { | ... | @@ -406,6 +406,38 @@ pub const Tokenizer = struct { |
| 406 | saw_at_sign, | 406 | saw_at_sign, |
| 407 | }; | 407 | }; |
| 408 | 408 | ||
| 409 | /// This is a workaround to the fact that the tokenizer can queue up | ||
| 410 | /// 'pending_invalid_token's when parsing literals, which means that we need | ||
| 411 | /// to scan from the start of the current line to find a matching tag - just | ||
| 412 | /// in case it was an invalid character generated during literal | ||
| 413 | /// tokenization. Ideally this processing of this would be pushed to the AST | ||
| 414 | /// parser or another later stage, both to give more useful error messages | ||
| 415 | /// with that extra context and in order to be able to remove this | ||
| 416 | /// workaround. | ||
| 417 | pub fn findTagAtCurrentIndex(self: *Tokenizer, tag: Token.Tag) Token { | ||
| 418 | if (tag == .invalid) { | ||
| 419 | const target_index = self.index; | ||
| 420 | var starting_index = target_index; | ||
| 421 | while (starting_index > 0) { | ||
| 422 | if (self.buffer[starting_index] == '\n') { | ||
| 423 | break; | ||
| 424 | } | ||
| 425 | starting_index -= 1; | ||
| 426 | } | ||
| 427 | |||
| 428 | self.index = starting_index; | ||
| 429 | while (self.index <= target_index or self.pending_invalid_token != null) { | ||
| 430 | const result = self.next(); | ||
| 431 | if (result.loc.start == target_index and result.tag == tag) { | ||
| 432 | return result; | ||
| 433 | } | ||
| 434 | } | ||
| 435 | unreachable; | ||
| 436 | } else { | ||
| 437 | return self.next(); | ||
| 438 | } | ||
| 439 | } | ||
| 440 | |||
| 409 | pub fn next(self: *Tokenizer) Token { | 441 | pub fn next(self: *Tokenizer) Token { |
| 410 | if (self.pending_invalid_token) |token| { | 442 | if (self.pending_invalid_token) |token| { |
| 411 | self.pending_invalid_token = null; | 443 | self.pending_invalid_token = null; |
| ... | @@ -1127,7 +1159,7 @@ pub const Tokenizer = struct { | ... | @@ -1127,7 +1159,7 @@ pub const Tokenizer = struct { |
| 1127 | state = .start; | 1159 | state = .start; |
| 1128 | result.loc.start = self.index + 1; | 1160 | result.loc.start = self.index + 1; |
| 1129 | }, | 1161 | }, |
| 1130 | '\t', '\r' => state = .line_comment, | 1162 | '\t' => state = .line_comment, |
| 1131 | else => { | 1163 | else => { |
| 1132 | state = .line_comment; | 1164 | state = .line_comment; |
| 1133 | self.checkLiteralCharacter(); | 1165 | self.checkLiteralCharacter(); |
| ... | @@ -1141,7 +1173,7 @@ pub const Tokenizer = struct { | ... | @@ -1141,7 +1173,7 @@ pub const Tokenizer = struct { |
| 1141 | result.tag = .doc_comment; | 1173 | result.tag = .doc_comment; |
| 1142 | break; | 1174 | break; |
| 1143 | }, | 1175 | }, |
| 1144 | '\t', '\r' => { | 1176 | '\t' => { |
| 1145 | state = .doc_comment; | 1177 | state = .doc_comment; |
| 1146 | result.tag = .doc_comment; | 1178 | result.tag = .doc_comment; |
| 1147 | }, | 1179 | }, |
| ... | @@ -1163,12 +1195,12 @@ pub const Tokenizer = struct { | ... | @@ -1163,12 +1195,12 @@ pub const Tokenizer = struct { |
| 1163 | state = .start; | 1195 | state = .start; |
| 1164 | result.loc.start = self.index + 1; | 1196 | result.loc.start = self.index + 1; |
| 1165 | }, | 1197 | }, |
| 1166 | '\t', '\r' => {}, | 1198 | '\t' => {}, |
| 1167 | else => self.checkLiteralCharacter(), | 1199 | else => self.checkLiteralCharacter(), |
| 1168 | }, | 1200 | }, |
| 1169 | .doc_comment => switch (c) { | 1201 | .doc_comment => switch (c) { |
| 1170 | 0, '\n' => break, | 1202 | 0, '\n' => break, |
| 1171 | '\t', '\r' => {}, | 1203 | '\t' => {}, |
| 1172 | else => self.checkLiteralCharacter(), | 1204 | else => self.checkLiteralCharacter(), |
| 1173 | }, | 1205 | }, |
| 1174 | .int => switch (c) { | 1206 | .int => switch (c) { |
| ... | @@ -1239,7 +1271,15 @@ pub const Tokenizer = struct { | ... | @@ -1239,7 +1271,15 @@ pub const Tokenizer = struct { |
| 1239 | fn getInvalidCharacterLength(self: *Tokenizer) u3 { | 1271 | fn getInvalidCharacterLength(self: *Tokenizer) u3 { |
| 1240 | const c0 = self.buffer[self.index]; | 1272 | const c0 = self.buffer[self.index]; |
| 1241 | if (std.ascii.isASCII(c0)) { | 1273 | if (std.ascii.isASCII(c0)) { |
| 1242 | if (std.ascii.isControl(c0)) { | 1274 | if (c0 == '\r') { |
| 1275 | if (self.index + 1 < self.buffer.len and self.buffer[self.index + 1] == '\n') { | ||
| 1276 | // Carriage returns are *only* allowed just before a linefeed as part of a CRLF pair, otherwise | ||
| 1277 | // they constitute an illegal byte! | ||
| 1278 | return 0; | ||
| 1279 | } else { | ||
| 1280 | return 1; | ||
| 1281 | } | ||
| 1282 | } else if (std.ascii.isControl(c0)) { | ||
| 1243 | // ascii control codes are never allowed | 1283 | // ascii control codes are never allowed |
| 1244 | // (note that \n was checked before we got here) | 1284 | // (note that \n was checked before we got here) |
| 1245 | return 1; | 1285 | return 1; |
src/AstGen.zig+4-2
| ... | @@ -10491,14 +10491,16 @@ fn strLitNodeAsString(astgen: *AstGen, node: Ast.Node.Index) !IndexSlice { | ... | @@ -10491,14 +10491,16 @@ fn strLitNodeAsString(astgen: *AstGen, node: Ast.Node.Index) !IndexSlice { |
| 10491 | var tok_i = start; | 10491 | var tok_i = start; |
| 10492 | { | 10492 | { |
| 10493 | const slice = tree.tokenSlice(tok_i); | 10493 | const slice = tree.tokenSlice(tok_i); |
| 10494 | const line_bytes = slice[2 .. slice.len - 1]; | 10494 | const carriage_return_ending: usize = if (slice[slice.len - 2] == '\r') 2 else 1; |
| 10495 | const line_bytes = slice[2 .. slice.len - carriage_return_ending]; | ||
| 10495 | try string_bytes.appendSlice(gpa, line_bytes); | 10496 | try string_bytes.appendSlice(gpa, line_bytes); |
| 10496 | tok_i += 1; | 10497 | tok_i += 1; |
| 10497 | } | 10498 | } |
| 10498 | // Following lines: each line prepends a newline. | 10499 | // Following lines: each line prepends a newline. |
| 10499 | while (tok_i <= end) : (tok_i += 1) { | 10500 | while (tok_i <= end) : (tok_i += 1) { |
| 10500 | const slice = tree.tokenSlice(tok_i); | 10501 | const slice = tree.tokenSlice(tok_i); |
| 10501 | const line_bytes = slice[2 .. slice.len - 1]; | 10502 | const carriage_return_ending: usize = if (slice[slice.len - 2] == '\r') 2 else 1; |
| 10503 | const line_bytes = slice[2 .. slice.len - carriage_return_ending]; | ||
| 10502 | try string_bytes.ensureUnusedCapacity(gpa, line_bytes.len + 1); | 10504 | try string_bytes.ensureUnusedCapacity(gpa, line_bytes.len + 1); |
| 10503 | string_bytes.appendAssumeCapacity('\n'); | 10505 | string_bytes.appendAssumeCapacity('\n'); |
| 10504 | string_bytes.appendSliceAssumeCapacity(line_bytes); | 10506 | string_bytes.appendSliceAssumeCapacity(line_bytes); |
test/compare_output.zig+9
| ... | @@ -535,4 +535,13 @@ pub fn addCases(cases: *tests.CompareOutputContext) void { | ... | @@ -535,4 +535,13 @@ pub fn addCases(cases: *tests.CompareOutputContext) void { |
| 535 | \\debug: free - len: 5 | 535 | \\debug: free - len: 5 |
| 536 | \\ | 536 | \\ |
| 537 | ); | 537 | ); |
| 538 | |||
| 539 | cases.add("valid carriage return example", "const io = @import(\"std\").io;\r\n" ++ // Testing CRLF line endings are valid | ||
| 540 | "\r\n" ++ | ||
| 541 | "pub \r fn main() void {\r\n" ++ // Testing isolated carriage return as whitespace is valid | ||
| 542 | " const stdout = io.getStdOut().writer();\r\n" ++ | ||
| 543 | " stdout.print(\\\\A Multiline\r\n" ++ // testing CRLF at end of multiline string line is valid and normalises to \n in the output | ||
| 544 | " \\\\String\r\n" ++ | ||
| 545 | " , .{}) catch unreachable;\r\n" ++ | ||
| 546 | "}\r\n", "A Multiline\nString"); | ||
| 538 | } | 547 | } |
test/compile_errors.zig+10
| ... | @@ -174,6 +174,16 @@ pub fn addCases(ctx: *TestContext) !void { | ... | @@ -174,6 +174,16 @@ pub fn addCases(ctx: *TestContext) !void { |
| 174 | }); | 174 | }); |
| 175 | } | 175 | } |
| 176 | 176 | ||
| 177 | { | ||
| 178 | const case = ctx.obj("isolated carriage return in multiline string literal", .{}); | ||
| 179 | case.backend = .stage2; | ||
| 180 | |||
| 181 | case.addError("const foo = \\\\\test\r\r rogue carriage return\n;", &[_][]const u8{ | ||
| 182 | ":1:19: error: expected ';' after declaration", | ||
| 183 | ":1:20: note: invalid byte: '\\r'", | ||
| 184 | }); | ||
| 185 | } | ||
| 186 | |||
| 177 | { | 187 | { |
| 178 | const case = ctx.obj("missing semicolon at EOF", .{}); | 188 | const case = ctx.obj("missing semicolon at EOF", .{}); |
| 179 | case.addError( | 189 | case.addError( |