compiler-research / CppInterOp

A Clang-based C++ Interoperability Library
Other
38 stars 20 forks source link

Add llvm 18 support #202

Closed mcbarton closed 3 months ago

mcbarton commented 4 months ago

This PR adds Clang-18 to the CI now that it has been released.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.63%. Comparing base (bd72e91) to head (59e47db).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/202/graphs/tree.svg?width=650&height=150&src=pr&token=7UWTYSVVT5&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research)](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) ```diff @@ Coverage Diff @@ ## main #202 +/- ## ======================================= Coverage 78.63% 78.63% ======================================= Files 8 8 Lines 3056 3057 +1 ======================================= + Hits 2403 2404 +1 Misses 653 653 ``` | [Files](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) | Coverage Δ | | |---|---|---| | [lib/Interpreter/DynamicLibraryManager.cpp](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research#diff-bGliL0ludGVycHJldGVyL0R5bmFtaWNMaWJyYXJ5TWFuYWdlci5jcHA=) | `75.12% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/202/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) | [Files](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) | Coverage Δ | | |---|---|---| | [lib/Interpreter/DynamicLibraryManager.cpp](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research#diff-bGliL0ludGVycHJldGVyL0R5bmFtaWNMaWJyYXJ5TWFuYWdlci5jcHA=) | `75.12% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/compiler-research/CppInterOp/pull/202/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research)
github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

mcbarton commented 4 months ago

@vgvassilev This PR is ready for review.

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

mcbarton commented 4 months ago

@vgvassilev It is passing all checks now.

mcbarton commented 3 months ago

@alexander-penev can you review this PR for me please?

vgvassilev commented 3 months ago

We will need to drop support for clang7. Our policy is to support the last 10 versions. That would require to clean up the ifdefs that mention clang 7.

vgvassilev commented 3 months ago

Can you edit the commit message to something like:

Add support for clang18.

Fixes #805.
mcbarton commented 3 months ago

Can you edit the commit message to something like:

Add support for clang18.

Fixes #805.

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.

mcbarton commented 3 months ago

We will need to drop support for clang7. Our policy is to support the last 10 versions. That would require to clean up the ifdefs that mention clang 7.

I will look for any ifdefs that mention clang 7 tomorrow and remove them.

vgvassilev commented 3 months ago

Can you edit the commit message to something like:

Add support for clang18.

Fixes #805.

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.

Dammit, no, I thought this is on the clad repository :(

vgvassilev commented 3 months ago

We will need to drop support for clang7. Our policy is to support the last 10 versions. That would require to clean up the ifdefs that mention clang 7.

I will look for any ifdefs that mention clang 7 tomorrow and remove them.

Please ignore that comment...

mcbarton commented 3 months ago

@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 https://github.com/compiler-research/xeus-cpp/pull/14 . This will also help troubleshoot any other changes which may be needed.