Fix some failing test cases from toml-test #87
Conversation
This file contains 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
cktan
reviewed
Oct 8, 2023
toml.c
Outdated
| int v = ('0' <= ch && ch <= '9') | ||
| ? ch - '0' | ||
| : (('A' <= ch && ch <= 'F') ? ch - 'A' + 10 : -1); | ||
| int v = -1; |
There was a problem hiding this comment.
maybe easier to do a ch=toupper(ch) and keep the original code
There was a problem hiding this comment.
Yeah, maybe; I changed it!
The ABNF has: HEXDIG = DIGIT / "A" / "B" / "C" / "D" / "E" / "F" But ABNF is case-insensitive by default, so \u007f should be identical to \u007F.
TOML spec says: Any Unicode character may be escaped with the `\xHH`, `\uHHHH`, or `\UHHHHHHHH` forms. The escape codes must be Unicode scalar values. Scalar value is defined by Unicode as: Any Unicode code point except high-surrogate and low-surrogate code points. In other words, the ranges of integers 0 to D7FF and E000 to 10FFFF inclusive. (See definition D76 in Section 3.9, Unicode Encoding Forms.) Whether it makes sense is another thing, but these values should be allowed.
Use \uXXXX escape sequences, instead of literal control characters.
e.g. day "0", hour "25", etc. Makes a number of toml-test tests pass.
Only lower-case is allowed, and without any underscores. Previously it would accept e.g.: INF i_nf
Previously it would accept these as valid: 1e_23 1_e2 1.2_e2
The following would be accepted as valid: 0x 0b 0 o -0b110 -0xff 0o77 +0b110 +0xff +0o77 0b_1 0x_1 0o_1
Explicitly list the toml-test files we know to fail, so it's easier to test for regressions and the like on changes. Can also add GitHub actions if you want. The stdex/float6 test was already failing on master. "make check" will run all the tests (closes cktan#83). Also remove the https://github.com/cktan/toml-spec-tests, based on @iarna/toml. These are no longer maintained, and toml-lang/toml-test contains everything that's in there.
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.
These are a few issues I found investigating failing test cases from toml-test, most are fairly simple low-handing fruit. There's still a few others that fail, but this already improves things.
Instead of sending 8 different pull requests it seemed easier to batch them in one; see the commit messages for more detailed explanations on the issues.
make checkwill now all run all the tests (unittest + stdex tests + toml-test), and will skip toml-test tests known to fail. Can also add GitHub action for this if you want.