Skip to content

[Core] Fix C++20-only compatibility issue in VtuOutput endianness detection#14326

Open
loumalouomega wants to merge 5 commits intomasterfrom
core/c++20-only-compatibility-issue-vtu-output
Open

[Core] Fix C++20-only compatibility issue in VtuOutput endianness detection#14326
loumalouomega wants to merge 5 commits intomasterfrom
core/c++20-only-compatibility-issue-vtu-output

Conversation

@loumalouomega
Copy link
Copy Markdown
Member

@loumalouomega loumalouomega commented Mar 27, 2026

📝 Description

This PR addresses a compatibility issue in the VTU output module related to endianness detection, ensuring that the code works correctly with C++17. The changes update the logic in kratos/input_output/vtu_output.cpp to avoid using C++20-only features, restoring compatibility with C++17 compilers.

(Devtools 11 in Rocky Linux does not fully supports C++20).

@roigcarlo we should revert devtools to version 11 in CI.

What is changed?

  • Added a standalone constexpr bool IsBigEndian() helper that uses a well-known union-based type-punning trick (reference) to detect endianness at compile time without any compiler-version guards or C++20 features.
  • Replaced the entire body of GetEndianness() with a single ternary delegating to IsBigEndian(), making the function C++17-compatible and compiler-agnostic.
bool IsBigEndian()
{
    // from: https://stackoverflow.com/a/1001373
    union {
        uint32_t i;
        unsigned char c[4];
    } bint = {0x01020304};

    return bint.c[0] == 1;
}

std::string GetEndianness()
{
    return IsBigEndian() ? "BigEndian" : "LittleEndian";
}

Why?

  • Restores compatibility with C++17 toolchains.
  • Keeps runtime behavior unchanged for supported little-endian and big-endian targets.
  • Preserves explicit failure on unsupported/unknown endianness configurations.

Validation

  • All existing VTU output tests (kratos/tests/test_vtu_output.py, kratos/mpi/future/tests/test_mpi_vtu_output.py) pass without modification.
  • No regressions observed in output files or test coverage.

🆕 Changelog

Files changed

  • kratos/input_output/vtu_output.cpp

Commits

@loumalouomega loumalouomega requested a review from a team as a code owner March 27, 2026 09:26
@loumalouomega loumalouomega added Kratos Core C++ Gcc Compilation FastPR This Pr is simple and / or has been already tested and the revision should be fast Refactor When code is moved or rewrote keeping the same behavior labels Mar 27, 2026
@loumalouomega loumalouomega enabled auto-merge March 27, 2026 09:26
@philbucher
Copy link
Copy Markdown
Member

this is used in cosimio, its independent of the standards version:

https://github.com/KratosMultiphysics/CoSimIO/blob/0006aaa6d5e4814f108ddd8733ed7a8ce4736a89/co_sim_io/sources/utilities.cpp#L62-L71

Not sure if its better though

@loumalouomega loumalouomega changed the title [Core] Fix endianness detection in vtu_output.cpp for C++17 compatibility [Core] Fix C++20-only compatibility issue in VtuOutput endianness detection Mar 27, 2026
@sunethwarna
Copy link
Copy Markdown
Member

ok, i thought we are fully in cpp20. Thats the reason I added it there... aparently we are not yet there :/

@loumalouomega
Copy link
Copy Markdown
Member Author

ok, i thought we are fully in cpp20. Thats the reason I added it there... aparently we are not yet there :/

Well, I thought we were, but GCC 11 is right now our limit, so we can just use some minor C++20 functions

@loumalouomega
Copy link
Copy Markdown
Member Author

Ok, looks like is fixed

@loumalouomega
Copy link
Copy Markdown
Member Author

@sunethwarna this is fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ Compilation FastPR This Pr is simple and / or has been already tested and the revision should be fast Gcc Kratos Core Refactor When code is moved or rewrote keeping the same behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants