Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix VtArray declaraions of extern template instantiations so that the… #2415

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

marktucker
Copy link
Contributor

@marktucker marktucker commented Apr 28, 2023

Fix VtArray declaraions of extern template instantiations so that the code in array.cpp will actually instantiate and export the templates as expected. Currently the templates are marked as extern even while compiling array.cpp so the compiler isn't actually instantiating anything. This problem seems to only apply to Windows builds.

Description of Change(s)

The existing code in vt/array.cpp doesn't actually generate exported symbols for some compilers as demonstrated here: https://godbolt.org/z/Gz1vc5eEe

The array.h file should be using the VT_API_TEMPLATE_CLASS macro so that when compiling array.cpp there is no confusion about whether code should be generated to instantiate the templates. In array.cpp VT_API must be added explicitly to the instantiation macro to ensure the generated symbols are exported.

Note that for many compilers there are many configurations here that all seem to work okay, either because of inlining of VtArry functions, or different interpretations of classes defined first as extern in the header, then non-extern in the cpp.

Thanks go to @e4lam for pointing this out.

  • [ X ] I have verified that all unit tests pass with the proposed changes
  • [ X ] I have submitted a signed Contributor License Agreement

… code

in array.cpp will actually instantiate and export the templates as expected.
Currently the templates are marked as extern even while compiling array.cpp
so the compiler isn't actually instantiating anything.
@tallytalwar
Copy link
Contributor

Filed as internal issue #USD-8286

@wi1k1n
Copy link

wi1k1n commented Aug 22, 2023

Had the exact same issue: clang on windows complaining about lld-link : error : undefined symbol: public: __cdecl pxrInternal_v0_23__pxrReserved__::VtArray<float>::VtArray<float>(void) in debug variant (strangelly was fine in release variant). applying changes from the commit solved the issue!

Many many thanks!

@moshev
Copy link

moshev commented Sep 11, 2023

I suspect this went unnoticed because MSVC does not (and will not) implement extern template, see https://developercommunity.visualstudio.com/t/c11-extern-templates-doesnt-work-for-class-templat/157868. So when compiling with cl.exe the template is simply instantiated each time, but when using a different compiler that follows the standard, it actually looks for the symbols and you get errors like:
error LNK2019: unresolved external symbol "public: float const & __cdecl pxrInternal_v0_22__pxrReserved__::VtArray<float>::operator[](unsigned __int64)const " (??A?$VtArray@M@pxrInternal_v0_22__pxrReserved__@@QEBAAEBM_K@Z) referenced in function
(posting it here so it's easier to google for others). Note that even though MSVC doesn't follow the standard and ignores the extern template declaration, the explicit instantiation is still exported in the DLL, so it's all fine - you can for example compile USD with MSVC and then compile and link a project that uses that USD with clang-cl.

Edit: @wi1k1n that is also why you're only seeing the error in a debug build. In a release build, the compiler (both MSVC and Clang) inlines most of VtArray.

@pixar-oss pixar-oss merged commit 94dfeb6 into PixarAnimationStudios:dev Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants