feat: support v21 and drop v8 #64
Conversation
shenxianpeng
added
enhancement
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for Clang v21 and removes support for Clang v8, updating the version support matrix and build configurations accordingly.
- Adds Clang v21 support with corresponding build configuration
- Removes Clang v8 support from CI workflows and documentation
- Updates support deprecation timeline to announce v8 removal by September 2025
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| include-cstdint-string-prior-to-using-uint8_t.patch | Removes patch file that was specific to Clang v8 compilation issues |
| README.md | Updates version support matrix to include v21 and removes v8, adds deprecation notice |
| .github/workflows/build.yml | Adds v21 build configuration and removes v8-specific build steps and patches |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| curl -LO https://github.com/llvm/llvm-project/releases/download/llvmorg-${version}/cfe-${version}.src.tar.xz | ||
| curl -LO https://github.com/llvm/llvm-project/releases/download/llvmorg-${version}/clang-tools-extra-${version}.src.tar.xz | ||
| - name: Get llvm-project | ||
| if: ${{ matrix.clang-version >= 9 || matrix.clang-version == '12.0.1' }} |
There was a problem hiding this comment.
The condition matrix.clang-version >= 9 will fail for version 21 because it's comparing a number to a string. Version 21 should be included in this condition, but the comparison logic needs to handle string versions properly or use numeric comparison.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughCI workflow updated to add clang 21 and remove clang 8, consolidating llvm-project download/unpack logic and tar extraction guards; README support matrix updated to drop v8 and note its planned removal; a Demangle header gained Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor GH as GitHub Actions
participant Workflow as build.yml
participant Matrix as clang-version job matrix
participant Fetch as Get/Unpack llvm-project
participant Build as build & test steps
GH->>Workflow: trigger CI
Workflow->>Matrix: expand clang versions (21,20,...,9,12.0.1)
loop per clang-version
Matrix->>Fetch: if version >= 9 or == "12.0.1"
Fetch->>Fetch: download release tarball (matrix.release)
alt Windows & clang-21
Fetch->>Fetch: extract with tar --ignore-missing-links (ignore failures)
else
Fetch->>Fetch: extract archive normally
end
Fetch->>Build: toolchain prepared
Build->>Build: compile & test
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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)
15-25: Typo: “Window 64” → “Windows 64” (three occurrences).Fix in all tool rows for consistency and professionalism.
Apply:
-| |Window 64 |✔️|✔️|✔️ |✔️|✔️|✔️ |✔️|✔️ |✔️ |✔️|✔️| ✔️|✔️| +| |Windows 64 |✔️|✔️|✔️ |✔️|✔️|✔️ |✔️|✔️ |✔️ |✔️|✔️| ✔️|✔️|Repeat for clang-tidy, clang-query, and clang-apply-replacements rows.
.github/workflows/build.yml (1)
71-73: Bump Windows build C++ standard to C++17
The/std:c++14flag forcesCMAKE_CXX_STANDARD=14, but LLVM requires at least C++17 (CMAKE_CXX_STANDARD 17+) and will fatal-error if lower; change to/std:c++17or drop the flag to let LLVM set it (github.com, reviews.llvm.org)
🧹 Nitpick comments (4)
README.md (1)
30-31: Note text and table are inconsistent about v8 timing.CI and this table already dropped v8 now (August 2025). Adjust to past tense with the exact date to avoid confusion.
-> Remove Support v8 (released in July 2019) by September 2025. +> Removed support for v8 (released in July 2019) in August 2025.Optionally also update the v7 line to past tense if removal already happened.
.github/workflows/build.yml (3)
100-112: Dead path for <9 can be dropped.Matrix no longer builds <9, so this unpack step will never run. Remove for simplicity and to avoid stale variables like extra-tar-args-cfe.
- - name: Unpack llvm-project - if: ${{ matrix.clang-version < 9 }} - shell: bash - run: | - version=${RELEASE##llvm-project-} - tar xf llvm-${version}.src.tar.xz - tar xf cfe-${version}.src.tar.xz ${{ matrix.extra-tar-args-cfe }} - tar xf clang-tools-extra-${version}.src.tar.xz - mkdir ${{ matrix.release }} - mv llvm-${version}.src ${{ matrix.release }}/llvm - mv cfe-${version}.src ${{ matrix.release }}/clang - mv clang-tools-extra-${version}.src ${{ matrix.release }}/clang-tools-extraAlso remove windows matrix key extra-tar-args-cfe if unused elsewhere.
88-99: If the new Demangle header patch is required, it’s not applied anywhere.AI summary mentions include-cstdint-string-prior-to-using-uint8_t.patch, but there’s no patch step. Add a guarded step before configure if needed.
+ - name: Patch Demangle includes (if needed) + if: ${{ matrix.clang-version >= 9 }} + shell: bash + run: | + if [ -f include-cstdint-string-prior-to-using-uint8_t.patch ]; then + patch -p0 < include-cstdint-string-prior-to-using-uint8_t.patch + fiPlease adjust -p level to match the patch.
176-181: Artifact naming and retention OK; consider bumping retention if downstream users need >24h.Optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/build.yml(1 hunks)README.md(2 hunks)include-cstdint-string-prior-to-using-uint8_t.patch(0 hunks)
💤 Files with no reviewable changes (1)
- include-cstdint-string-prior-to-using-uint8_t.patch
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: build (11, linux)
- GitHub Check: build (12.0.1, windows)
- GitHub Check: build (13, macosx)
- GitHub Check: build (12.0.1, macosx)
- GitHub Check: build (12, windows)
- GitHub Check: build (15, windows)
- GitHub Check: build (13, linux)
- GitHub Check: build (16, linux)
- GitHub Check: build (15, linux)
- GitHub Check: build (17, windows)
- GitHub Check: build (18, linux)
- GitHub Check: build (19, windows)
- GitHub Check: build (17, macosx)
- GitHub Check: build (18, macosx)
- GitHub Check: build (18, windows)
- GitHub Check: build (20, windows)
- GitHub Check: build (19, linux)
- GitHub Check: build (21, macosx)
- GitHub Check: build (21, linux)
- GitHub Check: build (20, linux)
🔇 Additional comments (5)
.github/workflows/build.yml (5)
23-23: Add clang 21 to matrix — good.Coverage now spans 21→9; aligns with the stated support window.
31-31: 19.x bump matches current 19.1 line — good.No action.
121-131: macOS CMake implicit link patch runs on all macOS versions — OK.No change required; just noting continued applicability with 21.
140-156: Smoke tests split per OS — good coverage.LGTM.
26-27: Tag llvmorg-21.1.0 verified: HTTP 200 confirms thellvm-project-21.1.0.src.tar.xzrelease is valid.
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 (1)
.github/workflows/build.yml (1)
50-55: Broken download/unpack for clang 10/9: missing “.src” suffix.The URL and extracted top-level dir rely on matrix.release. For 10/9 the suffix is missing, causing 404/tar errors.
- - clang-version: 10 - release: llvm-project-10.0.1 + - clang-version: 10 + release: llvm-project-10.0.1.src extra-cmake-args: '-DLLVM_ENABLE_Z3_SOLVER=OFF' - - clang-version: 9 - release: llvm-project-9.0.1 + - clang-version: 9 + release: llvm-project-9.0.1.src extra-cmake-args: '-DLLVM_ENABLE_Z3_SOLVER=OFF'
♻️ Duplicate comments (1)
.github/workflows/build.yml (1)
94-99: Fix type-mixed condition; it can error when clang-version is '12.0.1'.GitHub expressions may fail evaluating 'string >= number' even with ||. v8 is gone, so this condition is now redundant—make the step unconditional (simplest) or rework to a string-safe check.
- - name: Get llvm-project - if: ${{ matrix.clang-version >= 9 || matrix.clang-version == '12.0.1' }} + - name: Get llvm-project shell: bash run: | version=${RELEASE##llvm-project-}; version=${version%.src} curl -LO https://github.com/llvm/llvm-project/releases/download/llvmorg-${version}/${{ matrix.release }}.tar.xz
🧹 Nitpick comments (2)
.github/workflows/build.yml (2)
23-23: v21 added to matrix — looks good. Consider normalizing version types.LGTM for adding 21. Optional: quote all versions in the list to keep them consistently typed as strings and avoid type-mixing elsewhere.
- clang-version: [ 21, 20, 19, 18, 17, 16, 15, 14, 13, 12.0.1, 12, 11, 10, 9] + clang-version: [ '21', '20', '19', '18', '17', '16', '15', '14', '13', '12.0.1', '12', '11', '10', '9' ]
78-78: Remove unused key extra-tar-args-cfe.This matrix var isn’t referenced; drop it to reduce confusion.
- extra-tar-args-cfe: '--exclude=cfe-${version}.src/test/Driver/Inputs/*'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/build.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/build.yml (1)
26-27: Release tag llvmorg-21.1.0 verified. Download URL returns HTTP 200, confirming the llvm-project-21.1.0.src asset exists upstream.
.github/workflows/build.yml
Outdated
| # add --ignore-missing-links to ignore failure on v21 on windows | ||
| tar xf ${{ matrix.release }}.tar.xz --ignore-missing-links ${{ matrix.extra-tar-args }} |
There was a problem hiding this comment.
Scope --ignore-missing-links to Windows only to avoid macOS bsdtar errors.
bsdtar on macOS doesn’t support this GNU tar flag; keep it Windows-only.
- # add --ignore-missing-links to ignore failure on v21 on windows
- tar xf ${{ matrix.release }}.tar.xz --ignore-missing-links ${{ matrix.extra-tar-args }}
+ # add --ignore-missing-links to ignore failure on v21 on Windows
+ if [[ "${{ matrix.os }}" == "windows" || "${{ runner.os }}" == "Windows" ]]; then
+ tar xf ${{ matrix.release }}.tar.xz --ignore-missing-links ${{ matrix.extra-tar-args }}
+ else
+ tar xf ${{ matrix.release }}.tar.xz ${{ matrix.extra-tar-args }}
+ fi📝 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.
| # add --ignore-missing-links to ignore failure on v21 on windows | |
| tar xf ${{ matrix.release }}.tar.xz --ignore-missing-links ${{ matrix.extra-tar-args }} | |
| # add --ignore-missing-links to ignore failure on v21 on Windows | |
| if [[ "${{ matrix.os }}" == "windows" || "${{ runner.os }}" == "Windows" ]]; then | |
| tar xf ${{ matrix.release }}.tar.xz --ignore-missing-links ${{ matrix.extra-tar-args }} | |
| else | |
| tar xf ${{ matrix.release }}.tar.xz ${{ matrix.extra-tar-args }} | |
| fi |
🤖 Prompt for AI Agents
.github/workflows/build.yml around lines 102 to 103: the tar extraction adds the
GNU-only --ignore-missing-links flag unconditionally which breaks bsdtar on
macOS; change the workflow so that --ignore-missing-links is appended only when
running on Windows (e.g., guard it with a conditional on matrix.os == 'windows'
or build the tar command from a variable that includes the flag only for
Windows) so macOS runs use the plain tar extraction without that flag.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/build.yml (2)
23-55: Quote all clang-version entries to avoid type-mismatch in expressions.Mixing numeric (21, 20, …) and string ('12.0.1') versions makes comparisons brittle. Quote all versions for consistent typing and to prevent expression errors downstream. This also resolves the concern already raised about numeric comparisons failing.
- clang-version: [ 21, 20, 19, 18, 17, 16, 15, 14, 13, 12.0.1, 12, 11, 10, 9] + clang-version: [ '21', '20', '19', '18', '17', '16', '15', '14', '13', '12.0.1', '12', '11', '10', '9' ] @@ - - clang-version: 21 + - clang-version: '21' release: llvm-project-21.1.0.src - - clang-version: 20 + - clang-version: '20' release: llvm-project-20.1.0.src - - clang-version: 19 + - clang-version: '19' release: llvm-project-19.1.0.src - - clang-version: 18 + - clang-version: '18' release: llvm-project-18.1.5.src - - clang-version: 17 + - clang-version: '17' release: llvm-project-17.0.4.src - - clang-version: 16 + - clang-version: '16' release: llvm-project-16.0.3.src - - clang-version: 15 + - clang-version: '15' release: llvm-project-15.0.2.src - - clang-version: 14 + - clang-version: '14' release: llvm-project-14.0.0.src - - clang-version: 13 + - clang-version: '13' release: llvm-project-13.0.0.src - - clang-version: 12.0.1 + - clang-version: '12.0.1' release: llvm-project-12.0.1.src - - clang-version: 12 + - clang-version: '12' release: llvm-project-12.0.0.src - - clang-version: 11 + - clang-version: '11' release: llvm-project-11.1.0.src - - clang-version: 10 + - clang-version: '10' release: llvm-project-10.0.1 extra-cmake-args: '-DLLVM_ENABLE_Z3_SOLVER=OFF' - - clang-version: 9 + - clang-version: '9' release: llvm-project-9.0.1 extra-cmake-args: '-DLLVM_ENABLE_Z3_SOLVER=OFF'
94-99: Remove the brittle numeric comparison in the Get llvm-project step.Now that v8 is dropped, this guard is unnecessary and error-prone with mixed-type versions. Just always download.
- - name: Get llvm-project - if: ${{ matrix.clang-version >= 9 || matrix.clang-version == '12.0.1' }} + - name: Get llvm-project shell: bash run: | version=${RELEASE##llvm-project-}; version=${version%.src} curl -LO https://github.com/llvm/llvm-project/releases/download/llvmorg-${version}/${{ matrix.release }}.tar.xz
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
77-79: Remove unused matrix key extra-tar-args-cfe (or wire it up).
extra-tar-args-cfeis defined but never referenced. Drop it to reduce confusion or integrate it where intended.- extra-tar-args-cfe: '--exclude=cfe-${version}.src/test/Driver/Inputs/*'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/build.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (19, linux)
- GitHub Check: build (21, macosx)
- GitHub Check: build (20, linux)
| # ignore failure on v21 on windows | ||
| tar xf ${{ matrix.release }}.tar.xz ${{ matrix.extra-tar-args }} || true |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Don’t mask extraction failures; make Windows-only fallback explicit.
The comment says “ignore failure on v21 on windows,” but the command ignores all failures on all OSes. Remove || true and only add special handling on Windows. Also scope any GNU tar flags to Windows (not macOS) to avoid bsdtar errors.
- # ignore failure on v21 on windows
- tar xf ${{ matrix.release }}.tar.xz ${{ matrix.extra-tar-args }} || true
+ # Use Windows-only fallback; avoid GNU-only flags on macOS (bsdtar).
+ if [[ "${{ matrix.os }}" == "windows" || "${{ runner.os }}" == "Windows" ]]; then
+ # If needed, append GNU-only flags here; verify support on the runner.
+ tar xf "${{ matrix.release }}.tar.xz" ${{ matrix.extra-tar-args }}
+ else
+ tar xf "${{ matrix.release }}.tar.xz" ${{ matrix.extra-tar-args }}
+ fiIf Windows extraction still fails for v21, consider a guarded retry with 7-Zip:
+ if [[ "${{ matrix.os }}" == "windows" || "${{ runner.os }}" == "Windows" ]]; then
+ tar xf "${{ matrix.release }}.tar.xz" ${{ matrix.extra-tar-args }} || {
+ echo "tar failed; retrying with 7z";
+ 7z x "${{ matrix.release }}.tar.xz" -y && 7z x "${{ matrix.release }}.tar" -y
+ }
+ fi🏁 Script executed:
#!/bin/bash
# Show context around lines 80-120 in build.yml
sed -n '80,120p' .github/workflows/build.yml
# Search for definition of matrix.os
rg -nP '\bmatrix\.os\b' -C3 .github/workflows/build.yml
# Search for runner.os usage
rg -nP '\brunner\.os\b' -C3 .github/workflows/build.ymlLength of output: 5303
Make Windows-only tar fallback explicit and scope GNU-only flags
- Remove the unconditional
|| trueon the tar command and wrap it in a shell conditional onmatrix.os == 'windows'so extraction failures on macOS/Linux surface. - Ensure any GNU-specific flags in
matrix.extra-tar-argsare only declared for Windows in your matrix to avoid bsdtar errors on macOS.
Example refactor in .github/workflows/build.yml:
- name: Unpack llvm-project
shell: bash
run: |
if [[ "${{ matrix.os }}" == "windows" ]]; then
# mask failures only on Windows
tar xf "${{ matrix.release }}.tar.xz" ${{ matrix.extra-tar-args }} || true
else
tar xf "${{ matrix.release }}.tar.xz" ${{ matrix.extra-tar-args }}
fiOptional Windows 7-Zip retry for v21 failures:
if [[ "${{ matrix.os }}" == "windows" ]]; then
tar xf "${{ matrix.release }}.tar.xz" ${{ matrix.extra-tar-args }} || {
echo "tar failed; retrying with 7z";
7z x "${{ matrix.release }}.tar.xz" -y && 7z x "${{ matrix.release }}.tar" -y
}
fi🤖 Prompt for AI Agents
In .github/workflows/build.yml around lines 102-103, remove the unconditional
"|| true" from the tar extraction and instead make the fallback Windows-only by
wrapping the tar command in a shell conditional that checks if matrix.os ==
'windows'; on Windows keep the "|| true" (or the 7-Zip retry) so failures are
masked there, while on non-Windows run tar without "|| true" so extraction
failures surface. Also ensure any GNU-specific flags contained in
matrix.extra-tar-args are only set for the Windows matrix entries to avoid
bsdtar errors on macOS/Linux.
closes #63
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores