microsoft / DirectXMath

DirectXMath is an all inline SIMD C++ linear algebra library for use in games and graphics apps
https://walbourn.github.io/introducing-directxmath/
MIT License
1.52k stars 234 forks source link

Souce Code Annotation Language defines (sal.h) conflict with GCC <utility> header code #136

Closed sbd1138 closed 2 years ago

sbd1138 commented 2 years ago

I've discovered that the #defines in sal.h (included by DirectXMath.h) cause compilation issues with GCC utility headers (conflicting names in some std::move declarations, etc.).

Fortunately, this can be avoided by including the GCC headers before DirectXMath.h, but it's worth noting as it is not terribly obvious/easy to track down when you get compile errors by simply including utility.

To facilitate this, I've created by own wrapper include for DirectXMath, in which I explicitly include the problematic headers beforehand (as well as turn off various GCC warnings). eg:

#include <algorithm>
#include <utility>
#include "DirectXMath.h"

Thus far algorithm and utility have been the only two issues, but it's entirely conceivable there are other landmines yet to be stepped on.

I'm not sure there's a great solution for this aside from what I've done. Ideally it would be nice to eliminate having to hunt down the open-source sal.h by eliminating that requirement for non-Windows builds. However, I realize that presents a bit of a code explosion unless you were to add a level of indirection for all those SAL macros/defines...have DirectXMath macros for everything you use in the library, which on Windows resolve to the SAL macros, and on other platforms resolve to nothing.

walbourn commented 2 years ago

I'm aware of this issue. I ran into it when adding Windows Subsystem for Linux (WSL) support for DirectXTex, DirectXMesh, and UVAtlas which have to build with GCC 9.

The specific include order is really the best option I found to workaround this issue.

https://github.com/microsoft/DirectXMesh/pull/54

https://github.com/microsoft/DirectXTex/pull/208

https://github.com/microsoft/UVAtlas/pull/56

sbd1138 commented 2 years ago

The other worry being that these conflicting defines might introduce some other subtle issue elsewhere that isn't a compilation one. That's probably unlikely, but it does give one an uneasy feeling.