Fix compiler error on GCC 13

[kring]

This fixes the std::optional / std::variant related compiler errors that @lilleyse saw on GCC 13.

I was able to reproduce it on v13.1.0 on Ubuntu 22.04 (via WSL2) in the Release configuration, and this change fixed it for me. As far as I can tell, the previous code was valid and this is a GCC bug, but the code was a little unusual before and so the change here is an improvement.

Strangely enough, I can't seem to get it to fail to compile in the same way on the GH Actions ubuntu-24.04 runner with CC=gcc-13 and CXX=g++-13. But on that runner, after it compiles successfully, one of the tests fails:

192: Test command: /home/runner/work/cesium-native/cesium-native/build/CesiumNativeTests/cesium-native-tests "JsonValue::getSafeNumber() returns std::nullopt if narrowing conversion error would occur"
192: Working Directory: /home/runner/work/cesium-native/cesium-native/build/CesiumNativeTests
192: Test timeout computed to be: 1500
192: Filters: "JsonValue::getSafeNumber() returns std::nullopt if narrowing conversion error would occur"
192: Randomness seeded to: 3665553823
192: 
192: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
192: cesium-native-tests is a Catch2 v3.7.1 host application.
192: Run with -? for options
192: 
192: -------------------------------------------------------------------------------
192: JsonValue::getSafeNumber() returns std::nullopt if narrowing conversion error
192: would occur
192:   2^64 - 1 cannot be converted back to a double
192: -------------------------------------------------------------------------------
192: /home/runner/work/cesium-native/cesium-native/CesiumGltf/test/TestJsonValue.cpp:44
192: ...............................................................................
192: 
192: /home/runner/work/cesium-native/cesium-native/CesiumGltf/test/TestJsonValue.cpp:46: FAILED:
192:   REQUIRE( !value.getSafeNumber<double>().has_value() )
192: with expansion:
192:   false
192: 
192: ===============================================================================
192: test cases: 1 | 1 failed
192: assertions: 4 | 3 passed | 1 failed
192: 
192/498 Test #192: JsonValue::getSafeNumber() returns std::nullopt if narrowing conversion error would occur ..............***Failed    0.00 sec
test 193
        Start 193: JsonValue::getSafeNumberOrDefault() returns default if narrowing conversion error would occur

We've seen this particular test fail elsewhere before, but can't entirely explain it.

[lilleyse]

This doesn't seem to fix it completely, I'm still seeing compiler errors in GCC 13.2.0.

[kring]

That's unfortunate. @lilleyse can you give us a sense of how important it is to you to find a solution to this? Because it seems like it could end up being relatively time consuming.

[kring]

FWIW, I upgraded my WSL to Ubuntu 24.04:

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 24.04.1 LTS
Release:        24.04
Codename:       noble
$ gcc --version
gcc (Ubuntu 13.2.0-23ubuntu4) 13.2.0
Copyright (C) 2023 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Then did a clean build of cesium-native from main:

• git clean -d -f -x
• rm -rf ~/.ezvcpkg
• cmake -B build -S .
• cmake --build build

And everything built successfully.

[lilleyse]

I think CMAKE_BUILD_TYPE defaults to Debug, what if you try cmake -B build -S . -DCMAKE_BUILD_TYPE=Release

[lilleyse]

I pushed a change that ignores maybe-unitialized in those test files, maybe that's an ok workaround for now?

[kring]

Oh right, not sure how I forgot about that @lilleyse. So yes, when I build the Release configuration, I get the same error you're seeing. However, I don't get that error with the RelWithDebInfo configuration.
As another workaround, we could follow the lead of vcpkg and make our Release configuration equivalent to RelWithDebInfo. 😬
microsoft/vcpkg#9084

[kring]

I'm merging this. I'm not thrilled with needing to disable the warning, but the scope of it is extremely limited so it's the best alternative for now. Thanks @lilleyse!