scanner: accept octal and multi-byte escapes in rune literals (fix #28880) - #28882
JalonSolov merged 2 commits into
Conversation
…ang#28880) `validate_char_literal` (added in 3b7b5ee) splits a rune literal into characters to check it holds only one, but it gave every escape other than `\x`, `\u` and `\U` a single character after the backslash, and counted every byte escape as a character of its own. So `\141` was three characters and `\xe2\x98\x85` / `\342\230\205` were three and nine, although each is one character and strings accept all of them. The rune example in doc/docs.md, which documents both forms, stopped compiling, which is one of the two `check-markdown` failures on master. An octal escape is now three octal digits, as the parser's string decoder reads it, and a byte escape holding a UTF-8 lead byte takes the continuation byte escapes that complete its character. Incomplete, invalid and overlong sequences are still rejected. Code generation needed no change: with the validator fixed, the whole docs example runs and passes under tcc and gcc. Co-Authored-By: WOZCODE <contact@withwoz.com>
|
Additional local verification (Windows 11, tcc), comparing a compiler built from this branch against one built from the same tree with master's
The failure lists are identical except for Notes:
|
|
Local AI Findings: P1: Invalid UTF-8 sequences are accepted as valid rune literals. The new continuation check only verifies that continuation bytes are in
All four compile successfully with the PR compiler, despite the PR description stating that overlong and invalid sequences remain rejected. The lead-byte logic needs special-case bounds for The added tests cover |
The previous commit merged a UTF-8 lead byte escape with the escapes after it whenever each continuation byte was in 0x80..0xbf. That is not enough: the byte after E0, ED, F0 and F4 has a narrower range, so these were all accepted as one character (found by JalonSolov on vlang#28882): `\xe0\x80\x80` overlong `\xf0\x80\x80\x80` overlong `\xed\xa0\x80` surrogate `\xf4\x90\x80\x80` above U+10FFFF The commit message of that commit and the PR description said overlong and invalid sequences stayed rejected; that only held for the C0/C1 leads. Decode the code point and apply the range `check_string_escape` already enforces for `\u`/`\U` (at most U+10FFFF, no surrogates), plus the smallest code point each length may encode, which rules out overlong forms. With leads limited to C2..F4 and continuation bytes to 80..BF, that is exactly the well-formed set of Unicode Table 3-7. The new test checks the rule rather than examples: every lead byte C0..F7, every boundary of the second-byte ranges, and a valid and an invalid final byte, in hex and octal spelling, against `encoding.utf8.validate_str`. It fails on the previous commit, and removing any one of the four checks (shortest form, surrogate, maximum, continuation byte) makes it fail on that class. Also restore the `\u0061` case in the accept-list test: it had been written into the file as a plain `a`, so `\u` was never tested. Co-Authored-By: WOZCODE <contact@withwoz.com>
|
Thanks — confirmed and fixed in cf23b5a. You were right: the merge only checked that each continuation byte was The fix decodes the code point and applies the range The new test checks the rule rather than examples: every lead byte While fixing this I also found that the accept-list test had lost its One thing is still open: #28897. The |
|
If no further issues can we merge this fix? |
|
Local AI Findings: after most recent changes, no actionable issues found. |
Fixes #28880.
validate_char_literal(added in3b7b5eec98) splits a rune literal into characters to check that it holds only one. It knew the length of\x,\uand\Uescapes, but gave every other escape one character after the backslash, and it counted every byte escape as a character of its own:Each of these is one character, strings accept all of them, and
doc/docs.mddocuments both forms for runes — its example is one of the twocheck-markdownfailures on master.The change (
vlib/v/scanner/scanner.v)vlib/v/parser/parser.v:16396). With fewer, the backslash still escapes only the next character, as before.\xHHor octal byte escape holding a UTF-8 lead byte (0xc2–0xf4) takes the continuation byte escapes that complete its character, and the run counts as one character — but only when the bytes are one well-formed UTF-8 sequence (Unicode Table 3-7): every continuation byte in0x80–0xbf, and the decoded code point not overlong, not a surrogate and not above U+10FFFF (the same rangecheck_string_escapealready enforces for\u/\U). Otherwise nothing is merged and the literal is still rejected as more than one character, e.g.\xe0\x80\x80and\xf0\x80\x80\x80(overlong),\xed\xa0\x80(surrogate),\xf4\x90\x80\x80(above U+10FFFF),\xc0\x80(overlong lead).The first version (9c4c196) only checked the continuation-byte range and accepted those first four — found in review, fixed in cf23b5a.
Nothing else changes: the
\x/\u/\Uchecks, unknown-escape errors and the existing.vvfixtures (`\n\t`,`\nb`) behave as before.Code generation
No change needed. With only the validator fixed, the whole docs example compiles and every assertion passes under both tcc and gcc, including
`\141` == `a`and`\342\230\205`.bytes() == [u8(0xe2), 0x98, 0x85].Tests
vlib/v/scanner/scanner_test.v:test_char_literal_escapes_that_spell_one_character_are_accepted— octal,\x,\u,\U,\0, and 2-, 3- and 4-byte UTF-8 sequences spelled in hex, octal and a mix of both.test_char_literal_with_more_than_one_character_is_still_rejected—`\141b`, two ASCII byte escapes, an incomplete sequence, an invalid continuation byte, an overlong lead, and a complete sequence followed by an extra byte.test_char_literal_byte_escapes_are_one_character_only_when_well_formed_utf8— the rule itself rather than examples: every lead byteC0–F7× every boundary of the second-byte ranges × a valid and an invalid final byte, in hex and in octal spelling, checked againstencoding.utf8.validate_str. It fails on 9c4c196 (first on\xe0\x80\x80), and removing any one of the four checks (shortest form, surrogate, maximum, continuation byte) makes it fail on that class.Phase-R verified: with
scanner.vreverted to master, the first test fails onassert char_literal_diagnostics(source) == []; with the fix, both pass.Verified on this box (Windows 11, tcc + gcc 16.2.0)
With cf23b5a:
v test vlib/v/scanner/scanner_test.v→ OKv -silent vlib/v/compiler_errors_test.v→ 29 failed / 1689 passed / 7 skipped — exactly the same 29 failures (all undervlib/v/checker/tests/) as a compiler built from the same tree with master'sscanner.v.\xe0\x80\x80,\xf0\x80\x80\x80,\xed\xa0\x80,\xf4\x90\x80\x80) are rejected; the boundary characters\xe0\xa0\x80,\xed\x9f\xbfand\xf4\x8f\xbf\xbfcompile to U+0800, U+D7FF and U+10FFFF; the docs rune example still runs and passes.v fmt -verifyon both touched files → cleanWith 9c4c196:
v check-md doc/docs.mdno longer reports the rune example. What remains is thesqlexample, fixed separately in #28876, and one example that only fails on Windows (it passes in CI).Open: #28897. In
v -silent test vlib/v/scanner vlib/v/parser vlib/v/gen/c vlib/v/tests(VJOBS=20) the failure lists match master'sscanner.vexceptvlib/v/tests/comptime/comptime_on_generics_func_test.v, which crashed indriver.clone_int_string_map(#28897) in both runs with this branch's scanner (9c4c196: full run; cf23b5a: run stopped at 2204/2326) and in neither the one run with master's scanner nor 130 standalone compiles (60 under 20-way load and 5 alone, with each compiler). The file has no rune literals. Two against zero is not enough to say this change causes it, but it is not ruled out either.🧙 Built with WOZCODE