fix(parser): Parse more than utf8#1072
Merged
Merged
Conversation
d079d69 to
bc31ba6
Compare
Contributor
There was a problem hiding this comment.
VeIR Benchmarks
Details
| Benchmark suite | Current: fcbeb63 | Previous: 1971543 | Ratio |
|---|---|---|---|
add-fold-worklist/create |
1881000 ns (± 69769) |
2264000 ns (± 89652) |
0.83 |
add-fold-worklist/rewrite |
3599000 ns (± 57399) |
4027000 ns (± 79991) |
0.89 |
add-fold-worklist-local/create |
1863000 ns (± 27568) |
2225000 ns (± 106345) |
0.84 |
add-fold-worklist-local/rewrite |
3362000 ns (± 86170) |
3716000 ns (± 61682) |
0.90 |
add-zero-worklist/create |
1868000 ns (± 62051) |
2226000 ns (± 51046) |
0.84 |
add-zero-worklist/rewrite |
2235000 ns (± 20681) |
2600000 ns (± 51267) |
0.86 |
add-zero-reuse-worklist/create |
1576000 ns (± 55623) |
1889000 ns (± 90843) |
0.83 |
add-zero-reuse-worklist/rewrite |
1866000 ns (± 94180) |
2127000 ns (± 49138) |
0.88 |
mul-two-worklist/create |
1932500 ns (± 96028) |
2209000 ns (± 62608) |
0.87 |
mul-two-worklist/rewrite |
5095500 ns (± 185066) |
5620000 ns (± 248024) |
0.91 |
add-fold-forwards/create |
1896000 ns (± 90378) |
2279000 ns (± 74547) |
0.83 |
add-fold-forwards/rewrite |
2715000 ns (± 61249) |
3063000 ns (± 51863) |
0.89 |
add-zero-forwards/create |
1899000 ns (± 38004) |
2163000 ns (± 99039) |
0.88 |
add-zero-forwards/rewrite |
1790000 ns (± 24509) |
1940000 ns (± 16682) |
0.92 |
add-zero-reuse-forwards/create |
1551000 ns (± 32898) |
1902000 ns (± 49987) |
0.82 |
add-zero-reuse-forwards/rewrite |
1402000 ns (± 19942) |
1574000 ns (± 33890) |
0.89 |
mul-two-forwards/create |
1868000 ns (± 10040) |
2239000 ns (± 86176) |
0.83 |
mul-two-forwards/rewrite |
3279000 ns (± 71965) |
3723000 ns (± 102624) |
0.88 |
add-zero-reuse-first/create |
1583500 ns (± 55205) |
1837000 ns (± 97092) |
0.86 |
add-zero-reuse-first/rewrite |
10000 ns (± 2251) |
8000 ns (± 1821) |
1.25 |
add-zero-lots-of-reuse-first/create |
1568000 ns (± 57395) |
1924000 ns (± 83197) |
0.81 |
add-zero-lots-of-reuse-first/rewrite |
790500 ns (± 61209) |
801000 ns (± 13405) |
0.99 |
This comment was automatically generated by workflow using github-action-benchmark.
GZGavinZhao
reviewed
Jul 13, 2026
math-fehr
reviewed
Jul 13, 2026
math-fehr
left a comment
Collaborator
There was a problem hiding this comment.
Thanks a lot! I added a few comments, but otherwise all good.
0bb46b4 to
35e600d
Compare
Collaborator
Author
|
I've addressed all comments @math-fehr @GZGavinZhao. If you can have a check, and put the PR in to the merge queue please. |
math-fehr
approved these changes
Jul 14, 2026
math-fehr
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, that looks good to me, I'll just wait for @GZGavinZhao to review it then I can merge it!
GZGavinZhao
reviewed
Jul 14, 2026
GZGavinZhao
approved these changes
Jul 14, 2026
GZGavinZhao
left a comment
Collaborator
There was a problem hiding this comment.
Looks great! Thanks for implementing this. Just a small error message improvement suggestion, then should be good to merge!
35e600d to
603bbe1
Compare
\{\t,\n,"\} and their corresponding hex codes \{\09,\0A,\22\} are both printed as human-readable \{\t,\n,"\}.
603bbe1 to
fcbeb63
Compare
Collaborator
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(parse): Extend parser to parse more than UTF-8 (#954).
sqlite3.chas a char table that needs to be parsed. This PR allows non-UTF-8 to be parsed too. The three chars {\n,\t,"} are still printed as human-readable, even if their corresponding hex codes are parsed.The main change is that
Parser.parseStringLiteralnow returns aByteArray, rather than aString. Some of the previous uses (AttrParser,MlirParser) converted toByteanyway; these calculations have now gone.On the other hand,
UnitTest/Parserhas a wrapper to convert toStringfor comparison.