lighttransport / tinyusdz

Tiny, dependency-free USDZ/USDA/USDC library written in C++14
Other
492 stars 37 forks source link

Error when compiling in a C++20 project: std::expected is C++23 not C++20 #119

Closed newincpp closed 7 months ago

newincpp commented 7 months ago

Hi, I was trying to integrate tinyusdz in my (non-cmake) C++20 project and I was hitting this error:

/tmp/tinyusdz/src/nonstd/expected.hpp:229:16: error: ‘expected’ has not been declared in ‘std’
  229 |     using std::expected;

You can reproduce the error by setting CMAKE_CXX_STANDARD to 20 in CMakeLists.txt According to cppref: https://en.cppreference.com/w/cpp/utility/expected std::expected is a C++23 feature not a C++20 so enabling it when using C++20 is triggering an error.

The fix with the current code architecture would be relatively easy:

diff --git a/src/nonstd/expected.hpp b/src/nonstd/expected.hpp
index 4add6418..5541537c 100644
--- a/src/nonstd/expected.hpp
+++ b/src/nonstd/expected.hpp
@@ -113,10 +113,11 @@
 #define nsel_CPP14_OR_GREATER  ( nsel_CPLUSPLUS >= 201402L )
 #define nsel_CPP17_OR_GREATER  ( nsel_CPLUSPLUS >= 201703L )
 #define nsel_CPP20_OR_GREATER  ( nsel_CPLUSPLUS >= 202000L )
+#define nsel_CPP23_OR_GREATER  ( nsel_CPLUSPLUS >= 202302L )

 // Use C++20 std::expected if available and requested:

-#if nsel_CPP20_OR_GREATER && defined(__has_include )
+#if nsel_CPP23_OR_GREATER && defined(__has_include )
 # if __has_include( <expected> )
 #  define nsel_HAVE_STD_EXPECTED  1
 # else

With this tiny modification the code compile fine if C++20 is enabled.

Optionally for C++20 and onward features I would advice using the feature testing macro instead of the the cplusplus version. According to the same documentation page the macro emitted by the compiler you should test is: `cpp_lib_expected` more about C++20 feature testing: https://en.cppreference.com/w/cpp/feature_test This would require a bit more work on this file. Which is why I'm not making a pull request.

syoyo commented 7 months ago

Its a expected-lite module https://github.com/martinmoene/expected-lite, and there seems C++23 fix is included to there.

So updating expected-lite header file in TinyUSDZ would solve the issue .

syoyo commented 7 months ago

Updated expected-lite in this commit: https://github.com/syoyo/tinyusdz/commit/ac42ddeb186945a0b2ce3e936be9ae52f2b391d4

I thinks this should fix the build on C++20/23 compiler

newincpp commented 7 months ago

Oh woah you were fast. I'm sorry, I thought it was a code from tinyusdz and not an imported module. Updating it with your last commit indeed fixed the issue, thanks!