compiler: fix invalid UTF-8 and ternary codegen - #27636
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 928bb57de7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| if node.val.len == 1 { | ||
| clit := node.val[0] | ||
| if clit < 32 || clit == 92 || clit > 126 { |
There was a problem hiding this comment.
For single-byte literals above ASCII this now emits a C character constant such as '\377'; on the usual GCC/Clang targets with signed char, that constant evaluates to -1, so V rune/int uses like int(\xff) or comparing \xff to rune(255) regress even though casting back to u8 hides it. Emit a numeric/unsigned rune value for \x80-\xff instead of an ordinary C char constant.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f869123307
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else { | ||
| return replacement_rune |
There was a problem hiding this comment.
When this branch maps 0xf5-0xff to a replacement rune, the rest of this module still advances with utf8_char_len: len() adds 4 for these bytes and raw_index() adds 3 plus the loop increment. For invalid := [u8(0xf5), a].bytestr(), get_rune(invalid, 1) now returns a, but utf8.len(invalid) is 1 and utf8.reverse(invalid)/raw_index skip or drop the a. Please update the character-count/indexing paths to use the same validated length, i.e. one byte for this replacement case.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c26166ce6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| r, char_len := utf8_decode_rune(unsafe { &s.str[i] }, s.len - i) | ||
| runes << r |
There was a problem hiding this comment.
Fresh evidence beyond the earlier C-emission issue is that checker/comptime char-literal evaluation still calls value.runes()/expr.val.runes(), so this validation change makes a one-byte literal like \xff evaluate to U+FFFD in those paths while char_literal() emits 255. This breaks integer match bookkeeping, e.g. match x { \xff { } 255 { } } is no longer detected as a duplicate before C generation, and comptime evaluations can see 65533 instead of the byte value, so single-byte char literals need a byte-preserving path instead of the new UTF-8 replacement decoding.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f71d3125a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mut runes := []rune{cap: s.len} | ||
| for i := 0; i < s.len; i++ { | ||
| char_len := utf8_char_len(unsafe { s.str[i] }) | ||
| r, char_len := utf8_decode_rune(unsafe { &s.str[i] }, s.len - i) |
There was a problem hiding this comment.
Fresh evidence beyond the earlier checker/comptime paths is that the transformer still folds char-literal comparisons with node.left.val.runes()[0] / node.right.val.runes()[0] in vlib/v/transformer/transformer.v:1130-1131. After this replacement-decoding change, single-byte literals such as `\x80` and `\xff` both become U+FFFD during folding, so if `\x80` == `\xff` {} can be optimized to true before cgen's byte-preserving emission runs; please use a byte-preserving char-literal value helper for compiler literal evaluation paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cc9d2c28e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mut runes := []rune{cap: s.len} | ||
| for i := 0; i < s.len; i++ { | ||
| char_len := utf8_char_len(unsafe { s.str[i] }) | ||
| r, char_len := utf8_decode_rune(unsafe { &s.str[i] }, s.len - i) |
There was a problem hiding this comment.
Fresh evidence beyond the prior checker/transformer comments is that the wasm backend still lowers char literals through .runes()[0] (checked vlib/v/gen/wasm/gen.v:1267, vlib/v/gen/wasm/mem.v:218, and enum eval in vlib/v/gen/wasm/gen.v:1880). With this new decoder, a parsed byte literal such as `\xff` is a one-byte string whose first byte is invalid UTF-8, so those wasm paths now see U+FFFD/65533 while the C path and checker see 255; compiling code that uses high-byte char literals with -b wasm will generate the wrong constants. Please route backend literal evaluation through the byte-preserving char literal helper too.
Useful? React with 👍 / 👎.
1 participant
Summary
Tests
./vnew vlib/builtin/utf8_test.v./vnew vlib/encoding/utf8/utf8_util_test.v./vnew vlib/v/tests/return_match_expr_with_nest_match_expr_test.v./vnew vlib/builtin/string_iterator_test.v./vnew vlib/v/tests/char_literal_bytes_test.v./vnew -silent vlib/v/compiler_errors_test.v./vnew -silent test vlib/v/Fixes #27630
Fixes #27632