Skip to content

Move to C++17 #164

Description

@HugoFara

CMakeLists.txt:59 sets CMAKE_CXX_STANDARD 11 with STANDARD_REQUIRED ON. The build is now actively downgrading the compiler: GCC 16 reports __cplusplus 202002L by default, so ForeFire opts out of C++20 to opt into C++11.

It already builds and passes under C++17 and C++20, unchanged — no source changes, no shims:

Standard Build Unit tests runff
C++17 0 errors
C++20 0 errors 22/22, 2115 assertions KML + NetCDF pass

The bump is a two-line diff; the work is what it enables.

This needs a decision from @filippi

Discussed with @antonio-leblanc, who sees no obstacle. The person who can be impacted is @filippi, who may have pipelines pinned to C++11 — HPC sites and Meso-NH-adjacent builds are where an older toolchain is likely. This should not land without their agreement. What would help: which machines those pipelines run on, and the oldest compiler among them. GCC 5+ supports C++17 in full, so the bar is low, but the repository cannot answer this.

Meso-NH itself does not constrain the standard. It compiles no ForeFire C++ at all — it builds one C file (C_ForeFire_Interface.c), dlopens libForeFire.so, and resolves 17 extern "C" symbols through dlsym. There is no -std=c++ anywhere in the Meso-NH tree on 5.6 or 6.0. The build environments it runs in might still constrain it; the coupling does not.

The payoff is memory ownership

#159, #160 and the ownership gap behind #162's 5 MB of reported leaks are all one thing: raw owning pointers with no destructor discipline, across 200+ new/delete sites. unique_ptr for session- and domain-owned objects removes the class of bug rather than fixing instances. The session refactor keeps colliding with exactly this.

Also unlocked: std::optional for the SimulationParameters sentinel, currently the magic string "1234567890" (SimulationParameters.cpp:22); [[maybe_unused]] for a large share of #161's 314 -Wunused-parameter; std::filesystem for the path normalisation the HTTP path-traversal fix needs; structured bindings and if-init for the map lookups in DataBroker and Command.

17 rather than 20 is the conservative choice for a code that builds on HPC sites, and delivers all of the above. Nothing here argues against 20 later.

Suggested order

After #161, so the two diffs do not mix — the standard bump will itself change which warnings fire. Then convert ownership incrementally, starting with the objects Command::quit and ~FireDomain already try to manage.


Drafted by Claude Opus 5 from a codebase audit. Reviewed by a maintainer before filing.

EDIT: rewrote for human readability.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions