Skip to content

Fix cpp-static-checks: build TableGen headers before clang-tidy#2398

Open
bogdan-petkovic wants to merge 2 commits into
developfrom
users/bpetkovi/fix-cpp-static-check-missing-inc
Open

Fix cpp-static-checks: build TableGen headers before clang-tidy#2398
bogdan-petkovic wants to merge 2 commits into
developfrom
users/bpetkovi/fix-cpp-static-check-missing-inc

Conversation

@bogdan-petkovic
Copy link
Copy Markdown
Contributor

@bogdan-petkovic bogdan-petkovic commented Jun 2, 2026

Motivation

After PR #2356 moved the C++ format/tidy premerge checks from Jenkins to GitHub Actions, the lint job stopped working on any PR whose diff transitively reaches a TableGen-generated *.h.inc / *.cpp.inc (e.g. PR #2310). The Jenkins job ran on a fully built tree where these headers existed; the GHA job only ran cmake -S so clang-tidy hit file not found.

Technical Details

  • Add a cmake --build step that builds mlir-headers plus the orphan add_public_tablegen_target() declarations from mlir-hal (3) and rocMLIR (7) that are not chained into mlir-headers. New orphan targets must be appended to this list.
  • Disable precompiled headers on the lint configure to suppress spurious PCH diagnostics.

Adds ~7 minutes to the lint job wall time.

Test Plan

Re-run the cpp-static-checks job on a known previously-failing PR (#2310).

Test Result

  • PR CI

Submission Checklist

@bogdan-petkovic bogdan-petkovic requested a review from causten as a code owner June 2, 2026 18:42
@rocmlir-pr-reviewer rocmlir-pr-reviewer Bot added the modifies-ci-paths PR modifies the Claude review CI security perimeter; audit before applying claude-review label Jun 2, 2026
@rocmlir-pr-reviewer
Copy link
Copy Markdown

⚠️ This PR modifies the Claude-review CI security perimeter

The following files in this PR control whether and how the
claude-review workflow protects its LLM Gateway secrets at
runtime (see .github/workflows/CLAUDE_AUTO_REVIEW.md):

  • .github/workflows/ci.yml

Before applying the claude-review label on this PR, please:

  1. Audit the diff in these paths line-by-line. A malicious or
    accidental change could disable the --allowedTools
    restriction, the overlay step, the sanitizer, or the
    review/post job split -- any of which would expose the
    secrets in env to the PR-modified workflow.
  2. If the changes are legitimate and you want a Claude review,
    do NOT apply the claude-review label. Instead, run via
    Actions → Claude Auto Review → Run workflow and enter
    this PR's number. The dispatch path runs from the trusted,
    code-owner-approved version of the workflow on
    develop, so a malicious PR-side
    modification cannot affect the run.

(This banner is automated. The modifies-ci-paths label
will be removed automatically if a future push removes the
perimeter modifications. The Layer-3 in-workflow guard will
additionally fail the claude-review label-triggered run
on this PR if the label is applied while perimeter changes
are present.)

@bogdan-petkovic bogdan-petkovic self-assigned this Jun 2, 2026
Signed-off-by: bogdan-petkovic <bpetkovi@amd.com>
Signed-off-by: bogdan-petkovic <bpetkovi@amd.com>
@bogdan-petkovic bogdan-petkovic force-pushed the users/bpetkovi/fix-cpp-static-check-missing-inc branch from f2a076c to 897b2fd Compare June 2, 2026 18:57
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2398      +/-   ##
===========================================
+ Coverage    79.50%   81.96%   +2.45%     
===========================================
  Files          100      119      +19     
  Lines        31016    42672   +11656     
  Branches      4819     7082    +2263     
===========================================
+ Hits         24659    34972   +10313     
- Misses        4245     5070     +825     
- Partials      2112     2630     +518     
Flag Coverage Δ
gfx950 81.84% <ø> (?)
mfma ?
navi4x 81.81% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 116 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

modifies-ci-paths PR modifies the Claude review CI security perimeter; audit before applying claude-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant