Repository navigation
fix(lexer): codegen ? (quest) operator gives back in concatenations - #475
Pastalikek65 wants to merge 1 commit into
Conversation
The generated lexer\x27s quest (?, OpQuest) handling was fully greedy: the concatenation "A (X)? B" committed to consuming X and never retried the not-taken branch if B then failed. [A-Z][A-Z][A-Z]?T therefore could not match "EST" (the optional [A-Z] ate the trailing "T"). The runtime lexer (Go regexp) backtracks and matched fine, so generated and runtime lexers diverged. emitConcatWithQuest now generates a backtracking concat for quest sub-expressions: snapshot the position before the quest, try the greedy path, and on tail failure goto a fallback label that retries the tail from the not-taken position. Nested quests recurse; every closure still returns a single int, preserving the zero-allocation token path. - gen_lexer_cmd.go: emitConcatWithQuest + pre-pass that suppresses quest closures inlined by concats (previously dead code). - conformance_test.go: quest regression cases ([A-Z][A-Z][A-Z]?T on "EST", and the BASIC number [-+]?([0-9]*\.)?[0-9]+ covered by QuestNum). - basiclexer.go: regenerated. Fixes alecthomas#276.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eabe393321
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fmt.Fprintf(w, "fallback%d:\n", n) | ||
| w.Write([]byte("// quest not taken\n")) | ||
| w.Write([]byte("p = alt" + fmt.Sprint(n) + "\n")) | ||
| if len(tail) > 0 { | ||
| emit(tail, onFail) |
There was a problem hiding this comment.
When the optional operand can match multiple lengths, this fallback retries only the completely omitted branch. For example, the generated matcher for (?:a+)?a on aa first lets a+ consume both characters; after the trailing a fails, this code resets to the start and returns a one-character match. Go's regexp matcher instead gives one character back from a+ and matches both characters, so generated lexers can produce different token boundaries from stateful lexers. The taken branch must expose its internal alternatives before falling back to omitting the quest.
Useful? React with 👍 / 👎.
Closes #276
The generated lexer's
?(quest,OpQuest) handling was fully greedy: a concatenationA (X)? Bcommitted to consumingXand never retried the not-taken branch whenBsubsequently failed. So[A-Z][A-Z][A-Z]?Tcould not match"EST"— the optional[A-Z]greedily ate the trailing"T", and the literalTthen failed. The runtime lexer (Go's regexp engine) backtracks and matched correctly, so generated and runtime lexers diverged.Fix
cmd/participle/gen_lexer_cmd.go: newemitConcatWithQuestgenerates a backtracking concatenation whenever a quest appears as a direct sub-expression:int; backtracking uses only locals andgoto, so the zero-allocation token path is preserved.go buildwith "declared and not used").Regression coverage
lexer/internal/conformance/conformance_test.go:QuestGiveBack([A-Z][A-Z][A-Z]?Ton"EST"— the exact report) andQuestNumBoth([\+\-]?([0-9]*\.)?[0-9]+on12 -12.5 -.5 +.25), run against both the runtime lexer and a freshly generated one.lexer/internal/basiclexer.go: regenerated viascripts/regen-lexer; the BASIC Number rule exercises two nested quests.Both conformance peers pass; the stateful BASIC benchmark still yields the same 11101 tokens.