Skip to content

🚨 Run clang-tidy on MLIR subproject, switch to LLVM@20, and update to CMake >= 3.24 #879

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.

Sign up for GitHub

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

Jump to bottom
Merged
burgholzer merged 14 commits into from
Mar 23, 2025
Merged

🚨 Run clang-tidy on MLIR subproject, switch to LLVM@20, and update to CMake >= 3.24 #879

burgholzer merged 14 commits into from
Mar 23, 2025

Conversation

@burgholzer
Copy link
Member

@burgholzer burgholzer commented Mar 19, 2025 •
edited
Loading

Description

This pull request enables running clang-tidy for the MLIR subproject and updates the project to use LLVM version 20. This enhances code quality checks and aligns the project with the latest LLVM updates. It is also the same version currently shipped with CLion.

In addition, this PR raises the minimum supported CMake version to 3.24, which brings in several improvements:

  • Better FetchContent functionality (no more backwords-compatible workarounds)
  • Support for file sets which can be used to automatically install header files associated with a target.
  • CMAKE_VERIFY_INTERFACE_HEADER_SETS to enable build rules that verify all headers in header sets can be used on their own.

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

@burgholzer burgholzer self-assigned this Mar 19, 2025
@burgholzer burgholzer added dependencies Pull requests that update a dependency file code quality Code quality improvements continuous integration Anything related to the CI setup c++ Anything related to C++ code MLIR Anything related to MLIR labels Mar 19, 2025
@github-project-automation github-project-automation bot moved this to In Progress in MQT Core Mar 19, 2025
@codecov
Copy link

codecov bot commented Mar 19, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 84.04255% with 15 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
include/mqt-core/dd/Operations.hpp 42.8% 4 Missing ⚠️
src/dd/Simulation.cpp 0.0% 4 Missing ⚠️
src/ir/operations/StandardOperation.cpp 0.0% 3 Missing ⚠️
src/circuit_optimizer/CircuitOptimizer.cpp 60.0% 2 Missing ⚠️
src/ir/QuantumComputation.cpp 95.2% 1 Missing ⚠️
src/ir/operations/SymbolicOperation.cpp 50.0% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@burgholzer burgholzer added this to MQT Mar 19, 2025
@burgholzer burgholzer added this to the MQT Core milestone Mar 19, 2025
@github-project-automation github-project-automation bot moved this to In Progress in MQT Mar 19, 2025
@burgholzer burgholzer changed the title 🚨 Enable running clang-tidy for the MLIR subproject and switch to LLVM@19 🚨 Enable running clang-tidy for the MLIR subproject and switch to LLVM@20 Mar 20, 2025
@burgholzer
Copy link
Member Author

burgholzer commented Mar 21, 2025

@flowerthrower @ystade just venting my frustration here. Of course the LLVM set up would be making problems with clang-tidy.. it's not like that is actually part of the same monorepository..
I'll play around with this a little more and try to make it not complain, but at some point I am just going to put // IWYU pragma: ignore etc. in the respective parts and merge this to move it forward.

@burgholzer burgholzer changed the title 🚨 Enable running clang-tidy for the MLIR subproject and switch to LLVM@20 🚨 Run clang-tidy on MLIR subproject, switch to LLVM@20, and update to CMake >= 3.24 Mar 21, 2025
@burgholzer burgholzer added extensive-cpp-ci Adding this label indicates that extensive C++ CI runs should be started for this PR. Core Anything related to the Core library and IR labels Mar 21, 2025
burgholzer added 11 commits March 23, 2025 03:25
@burgholzer
⚗️ enable running clang-tidy for the MLIR subproject and switch to LL…
bb43960 
…VM@20

Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
🚨 address clang-tidy warnings
eb8ce24 
Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
✅ slightly expand random Clifford tests
874353b 
Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
⬆️ raise minimmum required CMake version to 3.24
6a6d24a 
the latest Ubuntu LTS version 24.04 ships with 3.28 so we should be good.

Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
🔧 set the C++17 compile time feature as a public feature on the MQT::…
249ab6b 
…CoreIR library

privately linking to the `project_options` does not propagte the feature requirement to the interface properties of the linked target, which is undesired. Everyone linking against our libraries will almost surely have to have C++ 17 support.

Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
🩹 fixes an incomplete include set in the QASM Parser
0b1cee8 
this was revealed as part of enabling `CMAKE_VERIFY_INTERFACE_HEADER_SETS`

Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
🚚 move the stray Definitions.hpp header to the ir submodule
f278f46 
also removes the legacy `QFRException` class and moves some methods from the `ir/Definitions` file to a more suitable place.

Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
🔧 refactor CMakeLists to use target_sources for header management and…
a3e5696 
… enable interface header verification

Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
🚨 another batch of clang-tidy fixes
8614da5 
Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
🎨 mark method as const
b9a307d 
Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
🔥 remove unused methods for printing DAGs
6f0d7be 
Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
✅ slighlty more testing to increase coverage
0a1b72e 
Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
🚨 fix clang-tidy warnings for MLIR-related clang-tidy problems
cb9963f 
Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
👷 testing the MLIR code base with LLVM@19 and LLVM@20
4642a92 
Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
Copy link
Member Author

burgholzer commented Mar 23, 2025

Alright, after the revelation that the project needs to be built in order for the MLIR files in the build directory to be generated, this seems to work now. I guess we will see in the upcoming MLIR PRs.
(I am still wondering if there is a way to more cleverly include files in various places so that the // IWYU pragma: keep statements are not necessary when the includes seem to be absolutely necessary for template instantiations.)

I am disregarding the patch coverage here, as these are mostly code paths that should never be exercised and merely exist out of caution.

@burgholzer burgholzer merged commit c5b8e11 into main Mar 23, 2025
63 of 64 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in MQT Core Mar 23, 2025
@burgholzer burgholzer deleted the update-cpp-linter branch March 23, 2025 11:28
@github-project-automation github-project-automation bot moved this from In Progress to Done in MQT Mar 23, 2025
This was referenced Mar 23, 2025
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@burgholzer burgholzer

Labels

c++ Anything related to C++ code code quality Code quality improvements continuous integration Anything related to the CI setup Core Anything related to the Core library and IR dependencies Pull requests that update a dependency file extensive-cpp-ci Adding this label indicates that extensive C++ CI runs should be started for this PR. MLIR Anything related to MLIR

Projects

None yet

Milestone

MQT Core

Development

Successfully merging this pull request may close these issues.

2 participants

@burgholzer