feat: update clang tools version support to include v22 and drop v9 and v10#78
feat: update clang tools version support to include v22 and drop v9 and v10#78shenxianpeng wants to merge 3 commits intomasterfrom
Conversation
WalkthroughThis PR updates CI and docs to expand tested Clang versions (adds 22 and 13–19, drops 10 and 9), tightens the llvm-project fetch condition to only 12.0.1, removes a Windows-specific trivially-copyable patch, and adjusts README deprecation notes and the support matrix. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (1)
12-29:⚠️ Potential issue | 🟠 MajorInconsistency: Table still includes v10 but PR objective is to drop v9 and v10.
The PR title states "drop v9 and v10" but the version support matrix still includes v10 (column headers and checkmarks). If v9 and v10 are being dropped in this PR, they should be removed from the table. If they're only being deprecated (not yet dropped), the table is correct but the PR title/objective should be clarified.
Additionally, Line 26 (
clang-apply-replacements | Linux 64) appears to have 14 checkmarks instead of 13, causing column misalignment:🔧 Suggested fix for Line 26
-| clang-apply-replacements| Linux 64|✔️|✔️|✔️|✔️|✔️|✔️|✔️ |✔️|✔️ |✔️ |✔️|✔️|✔️|✔️| +| clang-apply-replacements| Linux 64|✔️|✔️|✔️|✔️|✔️|✔️|✔️ |✔️|✔️ |✔️ |✔️|✔️|✔️|🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 12 - 29, The README matrix still contains column for v10 despite PR claiming to drop v9 and v10 and the "clang-apply-replacements | Linux 64" row has an extra checkmark causing column misalignment; update the table header to remove v9 and v10 columns (and corresponding checkmarks) or adjust the PR title if you intend only to deprecate, and fix the "clang-apply-replacements" Linux 64 row by removing the extra checkmark so it has the same number of columns as other rows; locate and edit the table block (look for the header row starting with "| Clang Tools |OS/Version |" and the row containing "clang-apply-replacements") to make the columns consistent..github/workflows/build.yml (1)
101-106:⚠️ Potential issue | 🔴 CriticalCritical:
Get llvm-projectstep condition restricts downloads to only version 12.0.1, causing build failures for all other versions.The condition
matrix.clang-version == '12.0.1'limits the tarball download to a single version. For all other versions in the matrix (22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11), the step is skipped, but the subsequentUnpack llvm-projectstep attempts to extract a non-existent tarball. Although the unpack command uses|| trueto suppress errors, the unpacked directory is never created. This causes the downstreamCmake,Build, andSmoke teststeps to fail when they reference${{ matrix.release }}/llvmor${{ matrix.release }}/build/bin.Remove or adjust the condition to allow the step to run for all versions in the matrix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yml around lines 101 - 106, The "Get llvm-project" step is incorrectly gated by the condition matrix.clang-version == '12.0.1', causing the tarball download to be skipped for other matrix entries and breaking downstream steps like "Unpack llvm-project", "Cmake", "Build", and "Smoke test"; remove or relax that condition so the "Get llvm-project" step always runs for every matrix entry (or change the condition to match all intended versions), ensuring the run block that computes version from RELEASE (version=${RELEASE##llvm-project-}; version=${version%.src}) and the curl download using ${{ matrix.release }} executes for each matrix item and produces the expected tarball for the subsequent "Unpack llvm-project" step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 33-39: The deprecation timeline in README.md is inconsistent with
current date: update the entries for "v7", "v8", "v9", and "v10" so they
accurately reflect their status—remove or move v7 (February 2025) and v8
(September 2025) entries if those versions are already dropped, and change the
v9 and v10 lines from future-tense "Drop support ... by March 2026" to
past-tense "Dropped support March 2026" (or remove them if already removed);
ensure you edit the exact strings "v7", "v8", "v9", and "v10" in the README to
either delete the obsolete lines, update their dates/status, or add a brief note
pointing to a changelog/release notes where the removal is documented.
---
Outside diff comments:
In @.github/workflows/build.yml:
- Around line 101-106: The "Get llvm-project" step is incorrectly gated by the
condition matrix.clang-version == '12.0.1', causing the tarball download to be
skipped for other matrix entries and breaking downstream steps like "Unpack
llvm-project", "Cmake", "Build", and "Smoke test"; remove or relax that
condition so the "Get llvm-project" step always runs for every matrix entry (or
change the condition to match all intended versions), ensuring the run block
that computes version from RELEASE (version=${RELEASE##llvm-project-};
version=${version%.src}) and the curl download using ${{ matrix.release }}
executes for each matrix item and produces the expected tarball for the
subsequent "Unpack llvm-project" step.
In `@README.md`:
- Around line 12-29: The README matrix still contains column for v10 despite PR
claiming to drop v9 and v10 and the "clang-apply-replacements | Linux 64" row
has an extra checkmark causing column misalignment; update the table header to
remove v9 and v10 columns (and corresponding checkmarks) or adjust the PR title
if you intend only to deprecate, and fix the "clang-apply-replacements" Linux 64
row by removing the extra checkmark so it has the same number of columns as
other rows; locate and edit the table block (look for the header row starting
with "| Clang Tools |OS/Version |" and the row containing
"clang-apply-replacements") to make the columns consistent.
| > Drop support for v10 (released July 2020) by March 2026. | ||
| > | ||
| > Drop support for v9 (released July 2019) by March 2026. | ||
| > | ||
| > Drop support for v8 (released July 2019) by September 2025. | ||
| > | ||
| > Drop support for v7 (released May 2019) by February 2025. |
There was a problem hiding this comment.
Deprecation timeline inconsistencies.
- v7 and v8 deprecation dates (February 2025 and September 2025) are in the past relative to March 2026 - if these versions have already been dropped, they shouldn't be listed here.
- The notes state v10 and v9 will be dropped "by March 2026" but if this PR is being merged now (March 2026), the table should reflect the removal, not just announce it.
Consider clarifying the timeline or removing entries for versions that have already been dropped.
🧰 Tools
🪛 LanguageTool
[style] ~37-~37: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...(released July 2019) by March 2026. > > Drop support for v8 (released July 2019) by ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~39-~39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...eased July 2019) by September 2025. > > Drop support for v7 (released May 2019) by F...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 33 - 39, The deprecation timeline in README.md is
inconsistent with current date: update the entries for "v7", "v8", "v9", and
"v10" so they accurately reflect their status—remove or move v7 (February 2025)
and v8 (September 2025) entries if those versions are already dropped, and
change the v9 and v10 lines from future-tense "Drop support ... by March 2026"
to past-tense "Dropped support March 2026" (or remove them if already removed);
ensure you edit the exact strings "v7", "v8", "v9", and "v10" in the README to
either delete the obsolete lines, update their dates/status, or add a brief note
pointing to a changelog/release notes where the removal is documented.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
README.md (1)
33-39:⚠️ Potential issue | 🟡 MinorDeprecation note is stale/inconsistent with current date.
At Lines 33–39, entries say “Drop support … by …” for deadlines that are already past (e.g., February 2025, September 2025, March 2026). Update to past-tense status (or remove) so docs match current support state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 33 - 39, The deprecation entries "Drop support for v10", "Drop support for v9", "Drop support for v8", and "Drop support for v7" are stale; update the README.md section so those lines reflect their current status by either converting them to past-tense (e.g., "Dropped support for v7 on February 2025") with the actual drop dates or removing the obsolete entries entirely; locate the strings "Drop support for v10", "Drop support for v9", "Drop support for v8", and "Drop support for v7" and replace them with accurate, consistent status lines (past-tense + date or removed) to match the current support state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 26: The Markdown table row for "clang-apply-replacements" has one extra
cell compared to the header (the offending row: "| clang-apply-replacements|
Linux 64|✔️|✔️|✔️|✔️|✔️|✔️|✔️ |✔️|✔️ |✔️|✔️|✔️|✔️|"); edit that row so it has
the exact same number of pipe-separated columns as the header (remove the extra
"✔️" cell or merge/sanitize adjacent cells) so the table columns align (keep the
same version column count as the header).
---
Duplicate comments:
In `@README.md`:
- Around line 33-39: The deprecation entries "Drop support for v10", "Drop
support for v9", "Drop support for v8", and "Drop support for v7" are stale;
update the README.md section so those lines reflect their current status by
either converting them to past-tense (e.g., "Dropped support for v7 on February
2025") with the actual drop dates or removing the obsolete entries entirely;
locate the strings "Drop support for v10", "Drop support for v9", "Drop support
for v8", and "Drop support for v7" and replace them with accurate, consistent
status lines (past-tense + date or removed) to match the current support state.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.mdwindows-clang-9-10-trivially-copyable-mismatch.patch
💤 Files with no reviewable changes (1)
- windows-clang-9-10-trivially-copyable-mismatch.patch
| | |Window 64 |✔️|✔️|✔️ |✔️|✔️|✔️ |✔️|✔️ |✔️ |✔️|✔️| ✔️| | ||
| | |macOS ARM64|✔️|✔️|✔️ |✔️|✔️|✔️ |✔️|✔️ |✔️ |✔️|✔️| ✔️| | ||
| | |macOS x86_64|✔️|✔️|✔️ |✔️|✔️|✔️ |✔️|✔️ |✔️ |✔️|✔️| ✔️| | ||
| | clang-apply-replacements| Linux 64|✔️|✔️|✔️|✔️|✔️|✔️|✔️ |✔️|✔️ |✔️|✔️|✔️|✔️| |
There was a problem hiding this comment.
Fix malformed Markdown table row at Line 26.
This row has one extra cell versus the header, so rendering will drop/truncate data. Keep the same number of version columns as the header (22..11).
Proposed fix
-| clang-apply-replacements| Linux 64|✔️|✔️|✔️|✔️|✔️|✔️|✔️ |✔️|✔️ |✔️|✔️|✔️|✔️|
+| clang-apply-replacements| Linux 64|✔️|✔️|✔️|✔️|✔️|✔️|✔️|✔️|✔️|✔️|✔️|✔️|📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | clang-apply-replacements| Linux 64|✔️|✔️|✔️|✔️|✔️|✔️|✔️ |✔️|✔️ |✔️|✔️|✔️|✔️| | |
| | clang-apply-replacements| Linux 64|✔️|✔️|✔️|✔️|✔️|✔️|✔️|✔️|✔️|✔️|✔️|✔️| |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 26-26: Table column count
Expected: 14; Actual: 15; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 26, The Markdown table row for "clang-apply-replacements"
has one extra cell compared to the header (the offending row: "|
clang-apply-replacements| Linux 64|✔️|✔️|✔️|✔️|✔️|✔️|✔️ |✔️|✔️ |✔️|✔️|✔️|✔️|");
edit that row so it has the exact same number of pipe-separated columns as the
header (remove the extra "✔️" cell or merge/sanitize adjacent cells) so the
table columns align (keep the same version column count as the header).
1 participant
Address part of cpp-linter/cpp-linter-action#397
Summary by CodeRabbit
Chores
Documentation