-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add llvm 18 support #202
Add llvm 18 support #202
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #202 +/- ##
=======================================
Coverage 78.63% 78.63%
=======================================
Files 8 8
Lines 3056 3057 +1
=======================================
+ Hits 2403 2404 +1
Misses 653 653
... and 1 file with indirect coverage changes
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
clang-tidy review says "All clean, LGTM! 👍" |
@vgvassilev This PR is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you run git-clang-format HEAD~
to make the bot happy?
clang-tidy review says "All clean, LGTM! 👍" |
clang-tidy review says "All clean, LGTM! 👍" |
@vgvassilev It is passing all checks now. |
@alexander-penev can you review this PR for me please? |
We will need to drop support for clang7. Our policy is to support the last 10 versions. That would require to clean up the |
Can you edit the commit message to something like:
|
I can change the commit message to "Add support for clang18" . We have no specific issue of support for clang 18 at the moment. I can create one and then add the fix message as you suggest. |
I will look for any ifdefs that mention clang 7 tomorrow and remove them. |
Dammit, no, I thought this is on the clad repository :( |
Please ignore that comment... |
@vgvassilev @alexander-penev after this PR is reviewed and merged I am going to add a xeus-cpp build to the end of the wasm CppInterOp CI jobs. This will allow me/you to test possible changes to get the correct extension in compiler-research/xeus-cpp#14 . This will also help troubleshoot any other changes which may be needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR adds Clang-18 to the CI now that it has been released.