folly/cli: eliminate double scan in cli_parse_args_from_content EOF path#2647
Open
darion-yaphet wants to merge 1 commit intofacebook:mainfrom
Open
folly/cli: eliminate double scan in cli_parse_args_from_content EOF path#2647darion-yaphet wants to merge 1 commit intofacebook:mainfrom
darion-yaphet wants to merge 1 commit intofacebook:mainfrom
Conversation
The code that handles a token still in `current` when content ends had
two independent backward scans:
1. A `while/pop_back` loop that stripped trailing spaces from the
processed string value (`current`).
2. A `for` loop that scanned `content` backward from EOF to recompute
the byte-span `length`, because `current` may have undergone escape
substitution and its size no longer matches the raw source bytes.
The root cause was that the main scan loop had no record of *where* in
`content` the last non-whitespace character of the current token sat.
Fix: add `token_end_offset` (byte index into `content` of the last
non-space character), updated in every branch that already updates
`token_end_line`/`token_end_col` — seven sites in total. The EOF path
can now compute `length = token_end_offset + 1 - token_start` in O(1),
removing the backward scan of `content` entirely. The `while/pop_back`
trim of `current` is kept (it handles the string value) but tightened
to the idiomatic `while (!x.empty() && cond(x.back()))` form.
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.
The code that handles a token still in
currentwhen content ends had two independent backward scans:while/pop_backloop that stripped trailing spaces from the processed string value (current).forloop that scannedcontentbackward from EOF to recompute the byte-spanlength, becausecurrentmay have undergone escape substitution and its size no longer matches the raw source bytes.The root cause was that the main scan loop had no record of where in
contentthe last non-whitespace character of the current token sat.Fix: add
token_end_offset(byte index intocontentof the last non-space character), updated in every branch that already updatestoken_end_line/token_end_col— seven sites in total. The EOF path can now computelength = token_end_offset + 1 - token_startin O(1), removing the backward scan ofcontententirely. Thewhile/pop_backtrim ofcurrentis kept (it handles the string value) but tightened to the idiomaticwhile (!x.empty() && cond(x.back()))form.