|
From: Matthias S. <zz...@ge...> - 2016-01-19 20:51:08
|
Am 17.01.2016 um 21:57 schrieb Matthias Schwarzott: > Am 17.01.2016 um 21:32 schrieb Florian Krohm: >> Hi Matthias, >> >>> >>> Third: The compiler is not allowed to optimize away the read access to >>> format. so we should get rid of the volatile. >>> >>> The simpler solution to cast format to void works for me. >>> I could today only check with gcc-5.3.0 and clang-3.7 but these two were >>> happy about this solution: >>> >>> (void)format; >>> >>> I hope msvc will like it too. >> >> Maybe msvc likes that. But static analysis tools may say that >> (void)format; is a statement with no effect which are known to be >> symptoms of bugs (sometimes). >> I was tempted for a moment to use: if (format) return; >> which would kill your warning as well, but then VALGRIND_PRINTF would be >> a function without effect and I know at least one checker that would >> complain about that. >> So I'm leaving the volatile in there. It'll shut up any compiler for good. >> > My only remaining point is that until now there was the guarantee that > absolutely no code is emitted when NVALGRIND is defined. > This is no longer valid with the commited solution. > > There is the different solution: > > Add the gcc attribute unused to the parameter. I hope this can be > applied for clang also, but I do not understand the current ifdef around > the declaration with __attribute__ (#if defined(__GNUC__) || > defined(__INTEL_COMPILER) && !defined(_MSC_VER)). > > The attribute code could also be cleaned by defining a macro > __attribute__ if the compiler does not support it (or defining a macro > __VALGRIND_ATTRIBUTE to not pollute the namespace). > > And then add some other code like that (without volatile) to silence MSVC: > > #if defined(_MSC_VER) > (void)format; > #endif > The attached patch implements exactly this (with just defining __attribute__ for not-supporting compilers). Matthias |