Skip to content

Fix metadirective and allocator parser regressions#48

Merged
ouankou merged 2 commits into
mainfrom
fix/ci-ctest-fragile-coverage
Apr 2, 2026
Merged

Fix metadirective and allocator parser regressions#48
ouankou merged 2 commits into
mainfrom
fix/ci-ctest-fragile-coverage

Conversation

@ouankou
Copy link
Copy Markdown
Owner

@ouankou ouankou commented Apr 2, 2026

Summary

This promotes the parser fixes currently living on fix/ci-ctest-fragile-coverage
into ouankou/ompparser main so rex can pin the submodule to origin/main
instead of carrying a branch reference.

What changed

  • fix structured BEGIN METADIRECTIVE / END METADIRECTIVE parsing so the end
    directive closes the block form rather than reusing the statement-associated
    metadirective production
  • preserve mapper / allocator string payloads with owned std::string storage
    instead of transient raw pointers coming from parser actions
  • reset clause-local string state explicitly before reuse so later clauses do not
    inherit stale parser state
  • teach pragma regeneration to print the paired begin metadirective form

Why

These regressions showed up downstream in rex as parser and unparse failures in
fragile OpenMP coverage. Merging them into main makes the fix available through a
stable submodule target and removes the need to keep following a non-main branch.

Validation

  • downstream rex builds against this branch
  • downstream CI-focused ctest coverage includes the fragile OpenMP paths that
    originally exposed these regressions

@ouankou
Copy link
Copy Markdown
Owner Author

ouankou commented Apr 2, 2026

@codex review

@ouankou
Copy link
Copy Markdown
Owner Author

ouankou commented Apr 2, 2026

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/ompparser.yy Outdated
@ouankou
Copy link
Copy Markdown
Owner Author

ouankou commented Apr 2, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

ℹ️ 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".

@ouankou ouankou merged commit 88f75f4 into main Apr 2, 2026
13 checks passed
@ouankou ouankou deleted the fix/ci-ctest-fragile-coverage branch April 2, 2026 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant