~Fix failing tests for invalid control characters in comments~ invalid #617
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
benchmark old ns/op new ns/op delta BenchmarkParseAll-8 3238 1941 -40.06%
When parsing strings, they can be referenced directly from the document when they don't contain escaped characters. This avoids paying to cost of allocating (and sometimes growing) the bytes buffer unecessarily.
* Use pointers instead of copying around ast.Node Node is a 56B struct that is constantly in the hot path. Passing nodes around by copy had a cost that started to add up. This change replaces them by pointers. Using unsafe pointer arithmetic and converting sibling/child indexes to relative offsets, it removes the need to carry around a pointer to the root of the tree. This saves 8B per Node. This space will be used to store an extra []byte slice to provide contextual error handling on all nodes, including the ones whose data is different than the raw input (for example: strings with escaped characters), while staying under the size of a cache line. * Remove conditional * Add Raw to track range in data for parsed values * Simplify reference tracking
Co-authored-by: Nabetani <takenori@nabetani.sakura.ne.jp>
* Reduces the public API. * Reuses optimized parsing functions. * Removes reliance on Google code under Apache license.
Inline call to hexToRune and uses specialized parsing, as found in encoding/json. Co-authored-by: Thomas Pelletier <thomas@pelletier.codes>
Co-authored-by: Thomas Pelletier <thomas@pelletier.codes>
Minimum supported version: Go 1.16.
When unmarshaling into a nested struct in a map, the value is not addressable. In that case, make a copy of it and modify it instead. Fixes pelletier#575
* parser: don't crash on unterminated table key Fixes pelletier#579 * parser: fix format of error returned by expect EOF was missing the format string and %U is not very human friendly.
When an invalid TOML expression ends with a comment before the end of file, the decode error would take a nil from scanComment, which is not part of the document. Fixes pelletier#588
This is required to support custom types. Fixes pelletier#590
…ier#601) RFC3399 allows for lowercase 't' and 'z' in date-time values. Fixes pelletier#600
Tests are hidden behind a "testsuite" build tag for now since many tests are failing. Use `go test -tags testsuite` to activate. Use `go generate` to regenerate toml_testgen_test.go. Co-authored-by: Thomas Pelletier <thomas@pelletier.codes>
When scanning comments, it makes better sense to halt scanning and immediately return if an illegal character is encountered while scanning. This can save on performance in the perverse case of an extremely long comment that has an early offending character. Related to: pelletier#613
Partially resolves: pelletier#613
Partially resolves: pelletier#613
Oddly enough, the test passes when it shouldn't. Partially resolves: pelletier#613
Oddly enough, the test passes when it shouldn't. Partially resolves: pelletier#613
|
Sorry, opened a PR against the wrong branch 😓 |
jidicula
changed the title
jidicula
changed the title
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.
Issue: #613
scanComment()to error out early if an illegal character is foundscanComment()Note that the NUL and US control characters weren't causing tests to fail prior to me making my changes... I took a quick look into why but couldn't crack why those tests weren't failing on my machine (
TestTOMLTest_Invalid_Control_CommentNullandTestTOMLTest_Invalid_Control_CommentUs).