Fix metadirective and allocator parser regressions#48
Conversation
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the metadirective directive in the OpenMP parser and refactors the handling of string parameters by transitioning from const char * to std::string. The changes include updated grammar rules for metadirectives and improved safety through null checks and explicit string clearing. A critical feedback point was raised regarding the uses_allocators clause, where secondStringParameter was not being cleared for enum-based allocators, potentially leading to stale state from previous user-defined allocators.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
This promotes the parser fixes currently living on
fix/ci-ctest-fragile-coverageinto
ouankou/ompparsermainsorexcan pin the submodule toorigin/maininstead of carrying a branch reference.
What changed
BEGIN METADIRECTIVE/END METADIRECTIVEparsing so the enddirective closes the block form rather than reusing the statement-associated
metadirective production
std::stringstorageinstead of transient raw pointers coming from parser actions
inherit stale parser state
begin metadirectiveformWhy
These regressions showed up downstream in
rexas parser and unparse failures infragile OpenMP coverage. Merging them into
mainmakes the fix available through astable submodule target and removes the need to keep following a non-main branch.
Validation
rexbuilds against this branchctestcoverage includes the fragile OpenMP paths thatoriginally exposed these regressions