prepare to C++20#33430
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThis PR upgrades the MuseScore project to C++20. The CMake configuration is updated to enforce C++20 as the language standard while disabling module scanning. A compile-time verification in main.cpp demonstrates C++20 features using consteval and static_assert. The RenderingContext struct is updated for C++20 compliance by removing its explicit default constructor declaration, allowing the compiler to generate an appropriate default. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/main.cpp (1)
40-40: 💤 Low valueConsider removing unused include.
The
<type_traits>header is included but not used in the C++20 compile-time check. The<concepts>header alone (line 39) serves as a sufficient C++20 sentinel.♻️ Proposed change
// C++20 check `#include` <concepts> -#include <type_traits> consteval int square(int n) { return n * n; } static_assert(square(5) == 25);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/main.cpp` at line 40, Remove the unused `#include` <type_traits> which is redundant because the C++20 compile-time sentinel already uses <concepts>; update src/app/main.cpp by deleting the <type_traits> include so only <concepts> remains (ensure any compile-time checks or concept usage in main.cpp still compile).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CMakeLists.txt`:
- Line 31: The CMakeLists currently unconditionally sets
CMAKE_CXX_SCAN_FOR_MODULES (symbol CMAKE_CXX_SCAN_FOR_MODULES) which requires
CMake 3.28+; either guard that set with a version check (inspect CMAKE_VERSION
or use if(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.28") / if(POLICY ...) and
only set CMAKE_CXX_SCAN_FOR_MODULES when the version is >= 3.28) or raise the
project minimum via the cmake_minimum_required(VERSION ...) entry to 3.28 if C++
modules are a core requirement; update the code around the existing CMake
minimum/version declaration and the line that sets CMAKE_CXX_SCAN_FOR_MODULES
accordingly.
---
Nitpick comments:
In `@src/app/main.cpp`:
- Line 40: Remove the unused `#include` <type_traits> which is redundant because
the C++20 compile-time sentinel already uses <concepts>; update src/app/main.cpp
by deleting the <type_traits> include so only <concepts> remains (ensure any
compile-time checks or concept usage in main.cpp still compile).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: abcb781b-f4fa-40a6-8636-f465f69e6abf
📒 Files selected for processing (4)
CMakeLists.txtsrc/app/main.cppsrc/engraving/playback/renderingcontext.hwf
💤 Files with no reviewable changes (1)
- src/engraving/playback/renderingcontext.h
| set(CMAKE_CXX_STANDARD 17) | ||
| set(CMAKE_CXX_STANDARD 20) | ||
| set(CMAKE_CXX_STANDARD_REQUIRED ON) | ||
| set(CMAKE_CXX_SCAN_FOR_MODULES OFF) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify CMAKE_CXX_SCAN_FOR_MODULES availability in CMake 3.22-3.27
echo "Checking if CMAKE_CXX_SCAN_FOR_MODULES could cause issues with CMake 3.22..."
echo "Searching for usage patterns and potential compatibility notes in the repository..."
# Search for any CMake version checks or notes about CMAKE_CXX_SCAN_FOR_MODULES
rg -n "CMAKE_CXX_SCAN_FOR_MODULES" --type cmake
# Check for any minimum CMake version comments or requirements
rg -n "cmake_minimum_required" --type cmake -A 2 -B 2Repository: musescore/MuseScore
Length of output: 2264
🌐 Web query:
CMake CMAKE_CXX_SCAN_FOR_MODULES minimum version requirement compatibility
💡 Result:
The CMAKE_CXX_SCAN_FOR_MODULES variable and the associated CXX_SCAN_FOR_MODULES target and source file properties were introduced in CMake 3.28 [1][2][3]. These features are part of CMake's support for C++20 modules [2][4]. When using CMake 3.28 or later, the policy CMP0155 governs the default behavior for scanning C++ sources for module dependencies [2][5]. Under the NEW behavior of CMP0155 (which is the default for projects specifying a minimum required CMake version of 3.28 or higher), CMake will automatically scan C++ sources in targets using C++20 or newer for module imports [2][4]. To disable this automatic scanning—which is often necessary for projects that use C++20 but do not use modules to avoid unnecessary build overhead—you can set the CMAKE_CXX_SCAN_FOR_MODULES variable to 0 (or OFF) in your CMakeLists.txt file [6][4][5]: set(CMAKE_CXX_SCAN_FOR_MODULES 0) It is recommended to set this near the top of your top-level CMakeLists.txt file and avoid placing it in the cache to prevent unintended side effects in projects included via FetchContent [6][4][5].
Citations:
- 1: https://cmake.org/cmake/help/latest/prop_sf/CXX_SCAN_FOR_MODULES.html
- 2: https://cmake.org/cmake/help/latest/policy/CMP0155.html
- 3: https://cmake.org/cmake/help/latest/variable/CMAKE_CXX_SCAN_FOR_MODULES.html
- 4: https://cmake.org/cmake/help/v4.3/manual/cmake-cxxmodules.7.html
- 5: https://man.archlinux.org/man/cmake-cxxmodules.7.en
- 6: https://cmake.org/cmake/help/git-stage/manual/cmake-cxxmodules.7.html
CMAKE_CXX_SCAN_FOR_MODULES requires CMake 3.28+, but minimum supported version is 3.22.
CMAKE_CXX_SCAN_FOR_MODULES was introduced in CMake 3.28. On CMake 3.22–3.27 (or 3.26–3.27 for macOS), this variable will be silently ignored without warnings, meaning the module scanning disable won't take effect. Consider conditionally setting this variable only when CMake 3.28+ is available, or updating the minimum required version to 3.28 if C++ modules are a core feature.
🧰 Tools
🪛 GitHub Actions: Check: Unit Tests (utest) / 0_run_tests.txt
[warning] 31-31: CMake deprecation warning: Compatibility with CMake < 3.10 will be removed; update VERSION argument min value (CMakeLists.txt:31 (cmake_minimum_required)).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CMakeLists.txt` at line 31, The CMakeLists currently unconditionally sets
CMAKE_CXX_SCAN_FOR_MODULES (symbol CMAKE_CXX_SCAN_FOR_MODULES) which requires
CMake 3.28+; either guard that set with a version check (inspect CMAKE_VERSION
or use if(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.28") / if(POLICY ...) and
only set CMAKE_CXX_SCAN_FOR_MODULES when the version is >= 3.28) or raise the
project minimum via the cmake_minimum_required(VERSION ...) entry to 3.28 if C++
modules are a core requirement; update the code around the existing CMake
minimum/version declaration and the line that sets CMAKE_CXX_SCAN_FOR_MODULES
accordingly.
1 participant
same as #29678
just small steps by steps