Skip to content
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 Clang 15 C++17 build with error tolerance #1764

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lucaam
Copy link

@lucaam lucaam commented Feb 5, 2025

This PR adds Clang 15 to the build matrix C++17 support and set it as non-voting (continue-on-error: true) and use the -k option to cmake continue going even if error occurs.

Copy link
Member

@joka921 joka921 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be the first step towards such a CI check.
I think for this to be very useful, we need also a postprocessor that reads the error output of the compilation, and lists the failing and non-failing .cpp fiiles that can and can't be compiled.
This can then be combined with an explicit whitelist (Files that we have already fixed) to really check, that we don't regress, and that we correctly track our progress.
But I assume, that this is your initial step, and as you get the output of the CI pipeline you can used that to develop the further tools.

Let me know if I can be of assistance.

CMakeLists.txt Outdated
Comment on lines 35 to 36
if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS "15.0.0")
MESSAGE(FATAL_ERROR "Clang++ versions older than 15.0 are not supported by QLever")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as this compilation doesn't work in fact, I would prefer having an additional
variable to be used here, such that this reads if (COMPILER_VERSION LESS 16 && !COMPILER_VERSION_CHECK_DEACTIVATED

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, way better solution, gonna push it soon

Comment on lines 50 to 56
- compiler: clang
compiler-version: 15
additional-cmake-options: "-DUSE_CPP_17_BACKPORTS=ON -DCMAKE_CXX_STANDARD=17 -DCMAKE_CXX_FLAGS='-ferror-limit=0'"
build-type: Debug
expensive-tests: false
continue-on-error: true
use-keep-going: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just started the pipeline, this would be an initial step to track the progress.

@sparql-conformance
Copy link

Copy link

codecov bot commented Feb 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.00%. Comparing base (db6825c) to head (c7faf13).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1764      +/-   ##
==========================================
+ Coverage   89.98%   90.00%   +0.01%     
==========================================
  Files         395      395              
  Lines       37693    37793     +100     
  Branches     4241     4257      +16     
==========================================
+ Hits        33919    34016      +97     
- Misses       2478     2479       +1     
- Partials     1296     1298       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

sonarqubecloud bot commented Feb 5, 2025

@lucaam
Copy link
Author

lucaam commented Feb 6, 2025

This can be the first step towards such a CI check. I think for this to be very useful, we need also a postprocessor that reads the error output of the compilation, and lists the failing and non-failing .cpp fiiles that can and can't be compiled. This can then be combined with an explicit whitelist (Files that we have already fixed) to really check, that we don't regress, and that we correctly track our progress. But I assume, that this is your initial step, and as you get the output of the CI pipeline you can used that to develop the further tools.

Let me know if I can be of assistance.

I think we could think about it in the future. Would you be open to a Python-based solution for this (we could include in a Github Workflow)?

@lucaam lucaam requested a review from joka921 February 6, 2025 08:53
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