Fix compilation issues when building USD using the C++20 standard #2605
Conversation
|
Filed as internal issue #USD-8601 |
mattyjams
force-pushed
the
pr/fix_compilation_errors_with_cxx20
branch
from
3ca32be to
7f7cae3
Compare
|
Freshening this with a rebase on the current dev branch, but I also added a commit for the |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mattyjams
force-pushed
the
pr/fix_compilation_errors_with_cxx20
branch
from
7f7cae3 to
92f80ca
Compare
|
Freshening with a rebase on the current dev branch (35dbce1). |
mattyjams
force-pushed
the
pr/fix_compilation_errors_with_cxx20
branch
from
92f80ca to
dac6168
Compare
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mattyjams
force-pushed
the
pr/fix_compilation_errors_with_cxx20
branch
from
dac6168 to
f10237c
Compare
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Hey @mattyjams, I'm looking to merge these changes in shortly. I had just one real question about the change in wrapPredicateExpression.cpp, would love to get your thoughts. Thanks!
…ceRegistry unique() was deprecated in C++17 and removed in C++20. Testing use_count() == 1 should be equivalent, though both may create thread safety concerns when used in a multithreaded environment. See the notes here for more detail: https://en.cppreference.com/w/cpp/memory/shared_ptr/use_count
…sourceRegistry and HdSt_SamplerObjectRegistry unique() was deprecated in C++17 and removed in C++20. Testing use_count() == 1 should be equivalent, though both may create thread safety concerns when used in a multithreaded environment. See the notes here for more detail: https://en.cppreference.com/w/cpp/memory/shared_ptr/use_count
…Index_Graph unique() was deprecated in C++17 and removed in C++20. Testing use_count() == 1 should be equivalent, though both may create thread safety concerns when used in a multithreaded environment. See the notes here for more detail: https://en.cppreference.com/w/cpp/memory/shared_ptr/use_count
…te patch The local patch to stb_image.h adds a "gamma" member to the previously unnamed struct used in the typedef of stbi__png. It also includes a default initializer to set the member to 0. When using C++17 or later, some compilers will emit an error that an unnamed class cannot be used within a typedef if it has a non-static data member with a default member initializer. In particular, this is the case with msvc: https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/c5208 In this case, the error can be avoided simply by giving the problematic struct a name. stb_image.h in hio was updated to make this change, and the patch file used to create it from the original source version was regenerated to ensure that the same change is applied if/when stb is upgraded in the future.
mattyjams
force-pushed
the
pr/fix_compilation_errors_with_cxx20
branch
from
f10237c to
7f1de71
Compare
Hey @sunyab! Thanks for taking a look at this one. Totally agree that with I just pushed a rebase of these changes on the current tip of the dev branch (20acbba). I also re-generated the One note though: When I tried to re-build to run the tests one more time, I'm actually hitting an unrelated compilation error in one of the And then again on line 112 of We seem to expect that to be available here: Did you run into that in your testing? Maybe you've got this addressed internally already as well? |
|
@mattyjams Oh interesting, no I haven't run into that in our test builds. What compiler version were you using for that one? |
Ah, hmm. This was on an M1 Mac, with MacOS 15.1.1, the latest Xcode (16.1) with clang 16.0.0, and Python 3.11.9. Here's the preamble of the USD build log (built through |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Ah got it, I'm still testing using Xcode 13. It does seem likely to be due to the removal of Oddly enough, it seems like that test is exercising functionality that isn't actually used in boost::python. The |
That also sounds great to me. :) Thanks again, @sunyab! |
Hello! :)
This may be a ways out on the road map still since the VFX reference platform hasn't made it there yet, but I took an initial crack at addressing some C++20 compilation issues and wanted to offer this small handful of changes that worked for me and still maintains compatibility with the currently chosen standard (C++14).
With the changes here, I was able to build USD with the C++ standard set forward to C++20 using the following patch:
I tested on the following configurations:
I unfortunately don't have a Linux environment I can test this with at the moment. Interestingly, I did not need to incorporate #2242 (which aims to address #2183), so that particular issue may be specific to using gcc.
Here are some details about the changes:
The first three commits replace the use of
std::shared_ptr'sunique()withuse_count() == 1instead.unique()was deprecated in C++17 and removed in C++20, and should be equivalent touse_count() == 1which is available back to C++11. I broke them out into individual commits updatinghd,hdSt, andpcp, but I'm also happy to create separate pull requests for each if that's preferable.I did want to flag the note on cppreference.com though:
I'm not sure whether that's of concern for the instances of
unique()currently in place, but switching touse_count()at least shouldn't be any less safe.The change in
hio/stbaddresses an issue where unnamed classes used in typedefs can no longer have a default member initializer. This is specific to thegammafield instb__pngwhich thehioversion adds. Since the affected file is sourced externally, I also updated the.patchfile used to producehio's version from the original.The last change in
sdfis somewhat perplexing, but the Boost Python wrapping forSdfPredicateExpression'sGetParseError()seems to fail to deduce properly when C++20 is enabled. I was able to get around this by adding an explicit static function to call the appropriate version, but I'd be curious to see whether anyone with stronger Boost and/or type deduction debugging-fu has a better idea. I tried bumping Boost to the latest version (1.82.0) as well, but that didn't seem to make a difference.I'll post the wall-of-text compilation error that was generated prompting the change separately so as not to (further) muddy the description here.
Hope these are helpful! Thanks!