Conversation
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a lexer state issue with uses_allocators clauses that contain nested parentheses. By introducing a parenthesis depth counter, uses_allocators_paren_depth, the lexer now correctly tracks nested structures and only exits the USES_ALLOCATORS_STATE when the top-level parentheses are closed. The changes are well-contained within src/omplexer.ll and include initializing, resetting, and updating the counter at the appropriate lexer rules. The logic correctly handles nested parentheses within allocator expressions and multiple allocators in a single clause. The implementation appears robust and solves the described problem effectively. I have not found any issues of high or critical severity.
Summary
Prevent
uses_allocators(...)lexing from exitingUSES_ALLOCATORS_STATEafter the first)by tracking parenthesis depth. This keeps the lexer in the correct state until the top‑level clause closes, so multiple allocators (and allocator traits) are tokenized correctly.Problem / Upstream bug
uses_allocatorscan legally contain multiple allocator entries, each optionally followed by allocator‑traits in parentheses. Example:The lexer previously popped
USES_ALLOCATORS_STATEon the first)it saw, which is inside the first allocator entry. That prematurely exits the clause, causing the remaining tokens to be lexed as generic expressions or otherwise misparsed. This breaks OpenMP 5.xuses_allocatorsparsing and leads to clause corruption during AST construction/unparse.Fix
uses_allocators_paren_depthcounter.USES_ALLOCATORS_STATE.(and only pop the state when the depth returns to zero.This matches the grammar structure of
uses_allocators(...)and preserves nested parentheses inside allocator trait lists.Tests
uses_allocatorsand comparing pragma round‑trip output; clause is now preserved without truncation.