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

build: start building with C++20 Standard #2019

Merged
merged 60 commits into from
Jan 7, 2025
Merged

build: start building with C++20 Standard #2019

merged 60 commits into from
Jan 7, 2025

Conversation

rprospero
Copy link
Contributor

@rprospero rprospero commented Nov 26, 2024

This PR starts compiling with the C++20 standard. This causes some knock-on effects

  • We no longer depend on the fmt formatting library
  • We need at least GCC 13
  • To retrieve GCC 13, we needed to upgrade to a more recent nixpkgs release, which brought on a patch upgrade of TBB
  • We've also upgraded LLVM to version 17, which also upgrades clang-format from version 13 to version 17
  • To get the appropriate clang for OS X, I've had to drop support for Intel Macs. To be honest, it should work on Macs running OS 14 or higher, but GitHub Actions don't provide Intel hardward for those OSes, so we wouldn't be able to test them.
  • The QC checks now use the commands from the nix shell, to ensure that upgrades are kept in sync. This has resulted in some format changed with updated version.

The most interesting files to look at are:

  • .github/workflows/qc/action.yml
  • CMakeLists.txt
  • src/gui/models/sortFilterProxy.h
  • src/items/serialisers.cpp
  • src/keywords/weightedModuleVector.cpp
  • src/templates/vector3.h
  • src/templates/vector4.h

A couple of notes to take while looking through the pr

  • There is not standard equivalent of fmt::print, so I replaced
    those calls with std::cout
  • std::format requires that the format string is known at
    compile time. This means that formats passed as a function or
    switched via a ternary expression cannot be used. The
    std::vformat function handles dynamic formats, though it
    requires an ugly wrapping of the parameters. For most of the
    ternary cases, I simply moved the conditional outside of the
    format call to regain the benefit of static format checking.
    Allegedly, C++26 will merge format and vformat.

I'll pull this out of draft once it compiles on all platforms and passes all tests.

@rprospero rprospero marked this pull request as ready for review December 5, 2024 09:44
src/base/enumOptions.h Outdated Show resolved Hide resolved
src/classes/atomTypeData.cpp Outdated Show resolved Hide resolved
src/classes/box.cpp Outdated Show resolved Hide resolved
src/generator/sequence.cpp Outdated Show resolved Hide resolved
src/keywords/weightedModuleVector.cpp Show resolved Hide resolved
src/main.cpp Show resolved Hide resolved
src/modules/energy/functions.cpp Show resolved Hide resolved
src/modules/geomOpt/functions.cpp Show resolved Hide resolved
tests/math/derivatives.cpp Outdated Show resolved Hide resolved
tests/math/expression.cpp Outdated Show resolved Hide resolved
@trisyoungs
Copy link
Member

Like it. Worth doing for the formatting changes in the PCG headers alone!

@rprospero
Copy link
Contributor Author

@trisyoungs I've moved in the copyright updates into this PR. I'm just looking to fix up the windows build. Or should we make that a separate PR, now that we at least have the QC working again.

@trisyoungs
Copy link
Member

@rprospero I think, if it turns out to be a quick / easy fix then wrap it in to this PR so we can get the whole CI rolling again. If it turns out to be more of an effort, split it.

@rprospero rprospero merged commit 0305b20 into develop Jan 7, 2025
8 checks passed
@rprospero rprospero deleted the Cpp20 branch January 7, 2025 14:30
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.

3 participants