Differential D91851
[compiler-rt] [emutls] Handle unused parameters in a compiler agnostic way Authored by mstorsjo on Nov 20 2020, 1:41 AM.
Details The MSVC specific pragmas disable this warning, but the pragmas themselves (when not guarded by any _MSC_VER ifdef) cause warnings for other targets, e.g. when targeting mingw. Instead silence the MSVC warnings about unused parameters by casting the parameters to void.
Diff Detail
Event TimelineComment Actions I don't have an objection to the change, but would like more details added to the commit message. It is unclear to me why the change is needed as clang does accept this I believe (and a quick test seems to indicate so as well). Comment Actions While clang does support those pragmas, it only recognizes them in MSVC(/itanium) mode, it doesn't in mingw mode, where I currently get these warnings: ../lib/builtins/emutls.c:185:9: warning: unknown pragma ignored [-Wunknown-pragmas]
#pragma warning(push)
^
../lib/builtins/emutls.c:186:9: warning: unknown pragma ignored [-Wunknown-pragmas]
#pragma warning(disable : 4100)
^
../lib/builtins/emutls.c:254:9: warning: unknown pragma ignored [-Wunknown-pragmas]
#pragma warning(pop)
^I can amend the commit message to something similar to this. Comment Actions Would be nice to actually mention that in the commit message and adjust the condition to indicate that (!defined(__MINGW32__) && !defined(__MINGW64__)). Comment Actions I think that's kind of inverts the logic IMO. We have an MSVC specific pragma in win32-generic code, and thus the ifdef should check for presence of _MSC_VER, not the absence of something else. (Also FWIW, in 64 bit cases __MINGW32__ still defined, just like _WIN32 is defined on 64 bit platforms as well.) Comment Actions ... and just to remind; clang in msvc mode also defines _MSC_VER, so it has the same effect as what you suggested (excluding the pragma in mingw mode), but with less inverted logic (and would also get rid of the warning for cygwin I think, if that target would end up in this codepath). Comment Actions clang -target x86_64-unknown-windows-itanium -x c -E - -dM <<< '' | grep _MSC_VER This is the reason that I prefer the "inverted" logic. Comment Actions Ah, I see. However, the itanium mode doesn't actually seems to support these pragmas either: $ cat pragma.c #pragma warning(push) #pragma warning(disable : 4206) #pragma warning(pop) $ bin/clang -target x86_64-windows-msvc -c pragma.c -Wall $ bin/clang -target x86_64-windows-gnu -c pragma.c -Wall pragma.c:1:9: warning: unknown pragma ignored [-Wunknown-pragmas] #pragma warning(push) ^ pragma.c:2:9: warning: unknown pragma ignored [-Wunknown-pragmas] #pragma warning(disable : 4206) ^ pragma.c:3:9: warning: unknown pragma ignored [-Wunknown-pragmas] #pragma warning(pop) ^ 3 warnings generated. $ bin/clang -target x86_64-windows-itanium -c pragma.c -Wall pragma.c:1:9: warning: unknown pragma ignored [-Wunknown-pragmas] #pragma warning(push) ^ pragma.c:2:9: warning: unknown pragma ignored [-Wunknown-pragmas] #pragma warning(disable : 4206) ^ pragma.c:3:9: warning: unknown pragma ignored [-Wunknown-pragmas] #pragma warning(pop) ^ 3 warnings generated. So with that in mind, the _MSC_VER check should actually be the right one, that silences the warning on itanium too?
| |||||||||