Repository navigation
Conversation
A syntax definition can make the parser loop on one line without consuming input in ways the existing loop guards do not detect. Callers had no way to stop such a parse (trishume#202). ParseState::parse_line_with_limit takes an Option<NonZeroUsize> token limit. Some(n) permits exactly n tokens; one more returns the new ParsingError::TokenLimitExceeded. None is unlimited, and parse_line delegates to the same code with no limit, so existing callers are unchanged. ParseState::parse_line_with_timeout takes a Duration and is built on the same counter: it reads the clock once every 256 tokens and returns ParsingError::TimeLimitExceeded once the deadline has passed. The counter is shared with the cross-line `fail` replay, so re-parsing buffered lines counts against the limit of the parse_line call that triggered it. ParsingError is #[non_exhaustive], so the two new variants are not a breaking change.
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.
Refs #202.
In #202, @trishume wrote:
This PR adds that, plus the token count it is built on.
What it adds
ParseState::parse_line_with_limit(line, syntax_set, limit: Option<NonZeroUsize>).Some(n)permits exactlyntokens. One more returnsParsingError::TokenLimitExceeded { limit }.Noneis unlimited.ParseState::parse_line_with_timeout(line, syntax_set, timeout: Duration). It uses the same counter and reads the clock once every 256 tokens (a private constant), so it can overshoot by up to 255 tokens. When the deadline has passed, it returnsParsingError::TimeLimitExceeded { timeout }.The count is the primitive and the
Durationis layered on top of it. A token count is deterministic, so a test can assert the exact boundary:"abcd\n"parses with a limit of 5 and fails with a limit of 4. A wall-clock limit cannot be asserted that way on a loaded machine. Callers who want reproducible behaviour, for example in their own tests, can use the count. Callers who think in time, as in #202, can use theDuration.Compatibility
parse_linekeeps its signature. It calls the same code with no limit, so no existing caller changes behaviour.ParsingErroris#[non_exhaustive], so the two new variants are not a breaking change.tests/snapshots/public-api.txtgains the two methods and the two variants, and nothing else.Details a reviewer may want to check
fail(Implement branch_point/branch/fail backtracking #614) count against the limit of theparse_linecall that triggered the replay. Without this, a replay could run without a limit.exec_patternandhandle_fail, which adds one argument to each. Both already had aclippy::too_many_argumentswarning before this change.Tests
Five new tests in
parsing::parser::tests. Four use a small syntax that produces one token per character:TokenLimitExceeded { limit: 4 },Nonereturns the same ops asparse_line,TimeLimitExceeded, a one-hour timeout parses, andDuration::MAXreturns the same ops asparse_line.The fifth uses a cross-line
branch_point: line 2 fits a limit of 20 on its own, but thefailreplays 100 characters of line 1, so the call returnsTokenLimitExceeded.cargo test --all-features: 194 lib (2 ignored), 8error_handling, 1public_api, 14 doctests pass. On master the lib count is 189.Why now
A second project has hit a hang of this kind: mdmost, a Markdown pager, hung on two lines of JavaScript. See my comment on #706: #706 (comment). #706 fixes that specific loop. This PR gives callers a limit for loops that no guard detects yet.