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

Remove sal.h requirement for non-MS builds #137

Closed sbd1138 closed 2 years ago

sbd1138 commented 2 years ago

As DirectXMath used just a handful of SAL annotations, it was pretty straightforward to create sal prepended versions of them which direct to the SAL macros on MS builds, and to nothing on non-MS builds. This eliminates the requirement for sal.h on non-Microsoft platforms (and the compilation issues with conflicting defines in GCC).

sbd1138 commented 2 years ago

This turned out to be a lot simpler than I expected. Obviously, a bunch of replacements, but overall way less obnoxious than I anticipated.

sbd1138 commented 2 years ago

Incomplete, retracting...

walbourn commented 2 years ago

I'm unlikely to accept this patch. This library is used internally at MSFT which has strong requirements around the use of SAL annotations.

The workaround for GCC should get you unblocked. I'll be sure to document this issue in the wiki and readme file.

For non-Windows platforms, there is an open source sal.h header as well--I configure this in the vcpkg port for DirectXMath for non-Windows platforms.

You are of course free to maintain a fork of the code if you feel strongly about this change for your workflow.

sbd1138 commented 2 years ago

Fair enough, I can understand that there are MSFT coding standards requirements. For what it's worth, I did finish these changes locally and it worked quite nicely (ended up being 10 defines). A much cleaner solution (IMO) than having to scare up a third party header and deal with future symbol conflicts on non-MS compilers.