Skip to content

Complementary CMake updates#20

Merged
OldDev78 merged 2 commits into
Redot-Engine:masterfrom
OldDev78:build-instructions
May 13, 2026
Merged

Complementary CMake updates#20
OldDev78 merged 2 commits into
Redot-Engine:masterfrom
OldDev78:build-instructions

Conversation

@OldDev78
Copy link
Copy Markdown
Contributor

@OldDev78 OldDev78 commented May 12, 2026

  • add build instructions
  • remove dead code (purge boost.ut)
  • add missing [[nodiscard]] to some math functions
  • tweak CMake default presets (enable compile_commands.json, set clang as the compiler).

Summary by CodeRabbit

  • Chores

    • Updated build configuration to use clang compiler with explicit C17/C++23 standards.
    • Enabled link-time optimization for Release builds.
    • Adjusted minimum C++ version requirement validation.
  • Documentation

    • Expanded README with updated work-in-progress notes and contribution invitation.
  • Refactor

    • Enhanced math library functions with compiler attributes for improved optimization and error detection.
    • Removed Boost.UT testing framework integration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 12, 2026

📝 Walkthrough

Walkthrough

The PR removes Boost.UT module and its main, updates CMake presets to prefer clang with C17/C++23 and related flags, enables IPO via CMake variable, adds [[nodiscard]]/constexpr/noexcept to several math templates, lowers the __cplusplus static_assert, and expands the README NOTE block.

Changes

Test Infrastructure & Build System Modernization

Layer / File(s) Summary
Remove Boost.UT module and entry point
engine/native/thirdparty/boost/ut.cppm, engine/native/thirdparty/boost/ut_main.cpp, cmake/Modules.cmake
Deleted the boost.ut module implementation and the main entry point; removed related commented bootstrap lines in cmake/Modules.cmake.
CMake default preset: explicit compilers & flags
CMakePresets.json
configurePresets[0].cacheVariables now sets CMAKE_C_COMPILER/CMAKE_CXX_COMPILER to clang/clang++, enforces C17/C++23 standards with extensions off, initializes fortify flags, enables CMAKE_CXX_SCAN_FOR_MODULES, and sets CMAKE_EXPORT_COMPILE_COMMANDS.
Enable IPO via CMake variable
cmake/Compiler.cmake
In Release when IPO/LTO is supported, sets CMAKE_INTERPROCEDURAL_OPTIMIZATION to TRUE instead of adding -flto link options.

Code Quality & Documentation

Layer / File(s) Summary
Math templates: attributes and qualifiers
engine/native/core/math/math.cppm
Added [[nodiscard]] to multiple exported templates and added constexpr/noexcept where applicable for sqr, abs, deg_to_rad, rad_to_deg, pow, lerp, cubic_interpolate, cubic_interpolate_in_time, bezier_interpolate, and bezier_derivative. Implementations unchanged.
C++ version check relax
engine/native/core/definitions/definitions.cppm
Lowered the static_assert C++ minimum from __cplusplus >= 202302L to __cplusplus >= 202207L.
README: expanded note
README.md
Replaced prior single-line W.I.P. with a multi-line NOTE clarifying the project and docs are incomplete and inviting contributions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Redot-Engine/DraconicEngine#19: Overlapping changes to Boost.UT wiring and cmake/Modules.cmake that comment out or remove boost_ut_main initialization.

Suggested reviewers

  • AR-DEV-1
  • mcdubhghlas
  • Arctis-Fireblight
  • JoltedJon

Poem

🐰 I nibble code in twilight's beam,

Modules pruned to sharpen the stream.
Clang and nodiscard, tidy and bright,
IPO whispers in Release night.
Hop on—docs ask for a helping bite.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Complementary CMake updates' is vague and does not clearly convey the specific changes made. While it partially relates to CMake modifications, it fails to highlight the main changes like compiler configuration, build optimizations, code cleanup, and math function improvements. Consider a more descriptive title such as 'Configure clang compiler, enable LTO, and enhance math functions with [[nodiscard]]' to clearly communicate the primary objectives.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@OldDev78 OldDev78 changed the title Clean up Complementary CMake updates May 12, 2026
mcdubhghlas
mcdubhghlas previously approved these changes May 13, 2026
JoltedJon
JoltedJon previously approved these changes May 13, 2026
@AR-DEV-1
Copy link
Copy Markdown
Contributor

@OldDev78 Could you add this check in Compiler.cmake?

include_guard(GLOBAL)

include(CheckIPOSupported)
check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT ERROR)

if (CMAKE_BUILD_TYPE STREQUAL "Release")
    if(IPO_SUPPORTED)
        message(STATUS "IPO / LTO enabled")
        add_link_options(-flto)
    else()
        message(STATUS "IPO / LTO not supported: <${ERROR}>")
    endif()
else()
    message(STATUS "IPO / LTO disabled")
    add_compile_definitions(DEBUG)
endif()

if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
    if (CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|AMD64")
        # TODO: Make SIMD level configurable or detect at runtime
        add_compile_options(-mavx2 -mfma)
    endif()
    if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
        if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 14.0)
            message(FATAL_ERROR "GCC version must be at least 14 for C++23 support")
        endif()
    endif()
    if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
        if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 18.0)
            message(FATAL_ERROR "Clang version must be at least 18 for C++23 support")
        endif()
    endif()
endif()

@OldDev78
Copy link
Copy Markdown
Contributor Author

@OldDev78 Could you add this check in Compiler.cmake?

include_guard(GLOBAL)

include(CheckIPOSupported)
check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT ERROR)

if (CMAKE_BUILD_TYPE STREQUAL "Release")
    if(IPO_SUPPORTED)
        message(STATUS "IPO / LTO enabled")
        add_link_options(-flto)
    else()
        message(STATUS "IPO / LTO not supported: <${ERROR}>")
    endif()
else()
    message(STATUS "IPO / LTO disabled")
    add_compile_definitions(DEBUG)
endif()

if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
    if (CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|AMD64")
        # TODO: Make SIMD level configurable or detect at runtime
        add_compile_options(-mavx2 -mfma)
    endif()
    if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
        if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 14.0)
            message(FATAL_ERROR "GCC version must be at least 14 for C++23 support")
        endif()
    endif()
    if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
        if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 18.0)
            message(FATAL_ERROR "Clang version must be at least 18 for C++23 support")
        endif()
    endif()
endif()

CMake already performs most compiler checks in the generation stage. If the compiler does not support that standard version flag - or c++ modules, for that matter - generation fails with an error message.

I can improve the static assert message already present in definitions.cppm that is also there to further gate the supported standard.

This did remind me to rewrite the link timer optimization flag as a (portable) property in CMake though.

@OldDev78 OldDev78 dismissed stale reviews from JoltedJon and mcdubhghlas via da16011 May 13, 2026 01:14
@OldDev78 OldDev78 requested review from JoltedJon and mcdubhghlas and removed request for mcdubhghlas May 13, 2026 01:14
@OldDev78 OldDev78 merged commit b1e6740 into Redot-Engine:master May 13, 2026
1 check passed
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.

4 participants