Skip to content

Drop llvm 18 support#785

Open
mcbarton wants to merge 18 commits intocompiler-research:mainfrom
mcbarton:Remove-clang-repl-llvm-18-support
Open

Drop llvm 18 support#785
mcbarton wants to merge 18 commits intocompiler-research:mainfrom
mcbarton:Remove-clang-repl-llvm-18-support

Conversation

@mcbarton
Copy link
Collaborator

This PR will drop llvm 18 support from CppInterOp.

@mcbarton mcbarton force-pushed the Remove-clang-repl-llvm-18-support branch from 3086ac8 to 3c1a272 Compare January 22, 2026 19:43
@mcbarton mcbarton force-pushed the Remove-clang-repl-llvm-18-support branch from 3c1a272 to ff4b763 Compare January 22, 2026 19:45
@codecov
Copy link

codecov bot commented Jan 22, 2026

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.46%. Comparing base (76d0600) to head (9f5800a).

Files with missing lines Patch % Lines
lib/CppInterOp/CppInterOp.cpp 50.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #785   +/-   ##
=======================================
  Coverage   79.46%   79.46%           
=======================================
  Files          11       11           
  Lines        4002     4002           
=======================================
  Hits         3180     3180           
  Misses        822      822           
Files with missing lines Coverage Δ
lib/CppInterOp/Compatibility.h 87.39% <ø> (ø)
lib/CppInterOp/CppInterOpInterpreter.h 83.18% <100.00%> (ø)
lib/CppInterOp/CppInterOp.cpp 87.84% <50.00%> (ø)
Files with missing lines Coverage Δ
lib/CppInterOp/Compatibility.h 87.39% <ø> (ø)
lib/CppInterOp/CppInterOpInterpreter.h 83.18% <100.00%> (ø)
lib/CppInterOp/CppInterOp.cpp 87.84% <50.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2026

clang-tidy review says "All clean, LGTM! 👍"

@mcbarton mcbarton force-pushed the Remove-clang-repl-llvm-18-support branch from 5db0c8c to ae8b326 Compare February 2, 2026 21:22
@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2026

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2026

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2026

clang-tidy review says "All clean, LGTM! 👍"

@mcbarton
Copy link
Collaborator Author

mcbarton commented Feb 3, 2026

In order to update the cling jobs to be based on roots llvm 20 an issue with Windows needs to be resolved. If you look at the ci you'll see the folllowing failure on Windows arm and Windows x86

[8](https://github.com/compiler-research/CppInterOp/actions/runs/21623972035/job/62319419973?pr=785#step:11:410)
  1: [ RUN      ] CppInterOpTest/InProcessJIT.Interpreter_CodeCompletion
1 : error : unable to make temporary file: invalid argument [D:\a\CppInterOp\CppInterOp\build\unittests\check-cppinterop.vcxproj]

and the following failures (looks like basically the same error hapenning for different tests) for Windows x86 only

 1: [ RUN      ] CppInterOpTest/InProcessJIT.FunctionReflection_JitCallAdvanced
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 23 (with alignment: 32 ) is needed but we only have 21.
   1: [ RUN      ] CppInterOpTest/InProcessJIT.TypeReflection_IsSmartPtrType
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 59 (with alignment: 72 ) is needed but we only have 41.
[45](https://github.com/compiler-research/CppInterOp/actions/runs/21623972035/job/62319419910?pr=785#step:11:547)
  1: [ RUN      ] CppInterOpTest/InProcessJIT.VariableReflection_DatamembersWithAnonymousStructOrUnion
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 23 (with alignment: 32 ) is needed but we only have 17.
[8](https://github.com/compiler-research/CppInterOp/actions/runs/21623972035/job/62319419910?pr=785#step:11:560)
  1: [ RUN      ] CppInterOpTest/InProcessJIT.VariableReflection_VariableOffsetsWithInheritance
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 567 (with alignment: 576 ) is needed but we only have 489.

@aaronj0 @Vipul-Cariappa do either of you have access to a Windows machine, and have the time to debug, so we can get a patch upstreamed to cling to enable us to update CppInterOps cling jobs?

@aaronj0
Copy link
Collaborator

aaronj0 commented Feb 5, 2026

In order to update the cling jobs to be based on roots llvm 20 an issue with Windows needs to be resolved. If you look at the ci you'll see the folllowing failure on Windows arm and Windows x86

[8](https://github.com/compiler-research/CppInterOp/actions/runs/21623972035/job/62319419973?pr=785#step:11:410)
  1: [ RUN      ] CppInterOpTest/InProcessJIT.Interpreter_CodeCompletion
1 : error : unable to make temporary file: invalid argument [D:\a\CppInterOp\CppInterOp\build\unittests\check-cppinterop.vcxproj]

and the following failures (looks like basically the same error hapenning for different tests) for Windows x86 only

 1: [ RUN      ] CppInterOpTest/InProcessJIT.FunctionReflection_JitCallAdvanced
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 23 (with alignment: 32 ) is needed but we only have 21.
   1: [ RUN      ] CppInterOpTest/InProcessJIT.TypeReflection_IsSmartPtrType
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 59 (with alignment: 72 ) is needed but we only have 41.
[45](https://github.com/compiler-research/CppInterOp/actions/runs/21623972035/job/62319419910?pr=785#step:11:547)
  1: [ RUN      ] CppInterOpTest/InProcessJIT.VariableReflection_DatamembersWithAnonymousStructOrUnion
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 23 (with alignment: 32 ) is needed but we only have 17.
[8](https://github.com/compiler-research/CppInterOp/actions/runs/21623972035/job/62319419910?pr=785#step:11:560)
  1: [ RUN      ] CppInterOpTest/InProcessJIT.VariableReflection_VariableOffsetsWithInheritance
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 567 (with alignment: 576 ) is needed but we only have 489.

@aaronj0 @Vipul-Cariappa do either of you have access to a Windows machine, and have the time to debug, so we can get a patch upstreamed to cling to enable us to update CppInterOps cling jobs?

Interesting, these tests should be running on ROOT's CI and seem to pass. I can take a look with our Windows machines, and I am currently working with CppInterOp + Cling. But if there are any other blockers, then I'd recommend keeping the cling jobs as is and landing this PR, and I can follow up once the cppyy migration is done (and builds with the latest Cling are stable). If not, I will need some time before I can get back to you with a fix for this windows issue

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

clang-tidy review says "All clean, LGTM! 👍"

@mcbarton
Copy link
Collaborator Author

mcbarton commented Feb 5, 2026

In order to update the cling jobs to be based on roots llvm 20 an issue with Windows needs to be resolved. If you look at the ci you'll see the folllowing failure on Windows arm and Windows x86

[8](https://github.com/compiler-research/CppInterOp/actions/runs/21623972035/job/62319419973?pr=785#step:11:410)
  1: [ RUN      ] CppInterOpTest/InProcessJIT.Interpreter_CodeCompletion
1 : error : unable to make temporary file: invalid argument [D:\a\CppInterOp\CppInterOp\build\unittests\check-cppinterop.vcxproj]

and the following failures (looks like basically the same error hapenning for different tests) for Windows x86 only

 1: [ RUN      ] CppInterOpTest/InProcessJIT.FunctionReflection_JitCallAdvanced
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 23 (with alignment: 32 ) is needed but we only have 21.
   1: [ RUN      ] CppInterOpTest/InProcessJIT.TypeReflection_IsSmartPtrType
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 59 (with alignment: 72 ) is needed but we only have 41.
[45](https://github.com/compiler-research/CppInterOp/actions/runs/21623972035/job/62319419910?pr=785#step:11:547)
  1: [ RUN      ] CppInterOpTest/InProcessJIT.VariableReflection_DatamembersWithAnonymousStructOrUnion
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 23 (with alignment: 32 ) is needed but we only have 17.
[8](https://github.com/compiler-research/CppInterOp/actions/runs/21623972035/job/62319419910?pr=785#step:11:560)
  1: [ RUN      ] CppInterOpTest/InProcessJIT.VariableReflection_VariableOffsetsWithInheritance
  1: Error in block allocation by ClingMemoryManager.
  1: Not enough memory was reserved for the current module.
  1: 567 (with alignment: 576 ) is needed but we only have 489.

@aaronj0 @Vipul-Cariappa do either of you have access to a Windows machine, and have the time to debug, so we can get a patch upstreamed to cling to enable us to update CppInterOps cling jobs?

Interesting, these tests should be running on ROOT's CI and seem to pass. I can take a look with our Windows machines, and I am currently working with CppInterOp + Cling. But if there are any other blockers, then I'd recommend keeping the cling jobs as is and landing this PR, and I can follow up once the cppyy migration is done (and builds with the latest Cling are stable). If not, I will need some time before I can get back to you with a fix for this windows issue

The Windows jobs are the only real blocker on this PR now. Once they are fixed, it should just be a case of releasing cling based on roots llvm 20, and then updating the ci in this PR and it will be ready to go. This PR only really becomes pressing once the PR updating CppInterOp to be compatible with llvm 22 is complete, since this PR frees up space in the cache for the llvm 22 jobs. I'll leave this PR left as is until you have a chance to investigate the discrepancy between CppInterOps ci and Roots ci.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2026

clang-tidy review says "All clean, LGTM! 👍"

@mcbarton mcbarton changed the title [wip] Drop llvm 18 support Drop llvm 18 support Feb 6, 2026
@mcbarton mcbarton requested review from Vipul-Cariappa, aaronj0 and vgvassilev and removed request for Vipul-Cariappa, aaronj0 and vgvassilev February 6, 2026 10:00
@mcbarton mcbarton marked this pull request as ready for review February 6, 2026 10:00
@mcbarton
Copy link
Collaborator Author

mcbarton commented Feb 6, 2026

@aaronj0 @vgvassilev @aaronj0 This PR is ready for review. It drops llvm 18 support. Given there is no release for cling based on roots llvm 20, this PR pins the ci to a particular commit for cling and roots llvm-20 (both whatever was the most recent commits yesterday). Once cling has a release based on roots llvm 20, this pinning could be removed.

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.

2 participants