Fix GCC -Wpedantic and -Wunused-but-set-variable warnings - #347
Open
Joel Kiptoo (Kiptoo-Deus) wants to merge 2 commits into
Open
Joel Kiptoo (Kiptoo-Deus) wants to merge 2 commits into
Joel Kiptoo (Kiptoo-Deus) wants to merge 2 commits into
Conversation
GCC reports "ISO C++ prohibits anonymous structs" for the anonymous structs in the XMFLOAT*, XMINT*, XMUINT* and PackedVector types. Clang's equivalent warnings are already suppressed by the existing "#pragma clang diagnostic" blocks, so add matching GCC blocks. The anonymous struct in XMMATRIX (_XM_NO_INTRINSICS_ only) is outside those blocks and also warns with clang, so wrap it for both compilers. In XMMatrixDecompose, only cc of the second XM3RANKDECOMPOSE is used, so mark aa and bb as unused. Fixes microsoft#342
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Comment on lines
+624
to
+626
| #endif | ||
| #if defined(__GNUC__) && !defined(__clang__) | ||
| #pragma GCC diagnostic push |
Collaborator
There was a problem hiding this comment.
Why not use #elif defined(__GNUC__) ?
There was a problem hiding this comment.
Makes sense, since clang defines __GNUC__ too, the #elif does the same job with less noise. Changed it everywhere.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #342.
GCC reports
ISO C++ prohibits anonymous structs [-Wpedantic]for the anonymous structs in theXMFLOAT*,XMINT*,XMUINT*and PackedVector types. Clang's equivalent warnings are already suppressed by the existing#pragma clang diagnosticblocks inDirectXMath.handDirectXPackedVector.h; this adds matching#pragma GCC diagnosticblocks (guarded bydefined(__GNUC__) && !defined(__clang__), so clang is unaffected) inside them.The anonymous struct in
XMMATRIX(_XM_NO_INTRINSICS_only) is outside those blocks, and clang warns about it too (-Wgnu-anonymous-struct,-Wnested-anon-types), so it gets its own push/pop for both compilers.In
XMMatrixDecompose, onlyccof the secondXM3RANKDECOMPOSEis used, soaaandbbare marked(void)to fix-Wunused-but-set-variable.Testing
Built a small program including
DirectXMath.h,DirectXPackedVector.h,DirectXCollision.handDirectXColors.h(system headers first,sal.hfrom dotnet/runtime, per the README) with-Wall -Wextra -Wpedantic -O2, for C++14, C++17 and C++20, with GCC 15.1 and Apple clang 21 on macOS. It callsXMMatrixDecomposeon a known scale/rotation/translation and checks the result.Warnings from this change's categories (
-Wpedantic,-Wunused-but-set-variable), same for all three standards:_XM_NO_INTRINSICS_All builds pass and the program returns the expected result. Anonymous structs in user code after the includes still warn with both compilers, so the suppression does not leak past the headers.
I don't have MSVC available to test with; the change only adds preprocessor blocks that MSVC skips, and the
(void)casts.Not included
GCC also reports
-Wstrict-aliasing("dereferencing type-punned pointer will break strict-aliasing rules"), which is not in this issue: 2 sites inDirectXPackedVector.inl(e.g.reinterpret_cast<float*>(&Result)[0]inXMConvertHalfToFloat) on the x86_64 and no-intrinsics paths, and 24 on the arm64 NEON path (DirectXMathVector.inl,DirectXMathConvert.inl,DirectXMath.h). These are real type-punning rather than style warnings, so I left them out of this PR; I'm happy to open a separate issue or PR if you'd like them addressed, and in which style (memcpyor otherwise).