Stephan Bergmann [EMAIL PROTECTED] writes:
Thorsten Behrens wrote:
-
Compiler Warning (level 3) C4686 'user-defined type' : possible
change in behavior, change in UDT
return calling convention
[...]
Agreed that such warnings should be disabled globally. (I also want
to disable warning C4347: behavior change: 'overload A' is called
instead of 'overload B' on CWS sb59, which is for example triggered
by some instances of std::minsal_Int32(a,b).)
Ok - will disable it then.
C4668 can potentially unearth real portability problems (that a try to
access an undefined preprocessor macro evaluates to '0' is a
convention possibly not shared by all c++ compilers of this universe)
This behavior is not a convention, but mandated by the C/C++
standards. Still, in all the cases I encountered so far where MSC
issued this warning, it either (a) unearthed really broken code like
#if STRCODE==sal_Unicode in tools/source/string/strimp.cxx:1.7, (b)
was easy to either fix the offending #if FOO to something like #ifdef
FOO (which appeared to be the intended code, and the original author
had merely been sloppy), or (c) was easy to wrap the offending
(external) header in some disable-warnings #pragma (which was needed
anyway, as the external header also produced other warnings). Thus, I
am reluctant to disable this warning globally.
- unfortunately, it's implicitely relied upon by boost's
BOOST_WORKAROUND macro (boost/workaround.hpp). Since no other compiler
warns about that, I've no idea how to fix BOOST_WORKAROUND, and this
macro's use is quite abundant in boost, I'd also suggest disabling
it. This could be temporary, until upstream boost has
fixed/workarounded this.
Would it work to disable that warning locally within boost headers?
In principle, yes - only that there are currently ~25 files to clobber
with this (but see below).
Last, but not least there's a really annoying side effect of gcc's
-Wshadow warning: when declaring functions that overload one of the
gcc builtin functions, you get the following warning issued:
stl/stl/_complex.h:782: warning: shadowing built-in function `int
std::abs(int)'
Switching off -Wshadow cures this, as well as disabling gcc's
builtins
via -fno-builtin. But both prolly aren't changes we want to do
persistently for gcc. Any other ideas?
We already sprinkled our STLport patches with pragmas to disable
warnings locally within those headers. Seems this is just yet another
place where we need to do that.
Hm. I'm not sure whether this is an approach that scales. Admittedly,
the ~180 cases in STLport-4.0.patch are generated automatically - but
is anything of that going to be upstreamed?
There's of course a fine line between fixing obviously incorrect code,
and mutilating it to the likings of specific compiler's warning
sets. The latter does not make the code more portable, and
significantly increases the chance of build breakage on the platforms
not built (unless there are zero-cost mechanisms to build on _all_
werror=t platforms - c.f. build bots).
I'm not suggesting that C4668 falls into this category (-Wshadow
clearly does not), but I'm convinced that we must agree on a set of
warnings that's enforced (werror=t), and a superset that can
optionally be used by the developer to improve code quality. For the
former, we should err on the safe side, the latter could also contain
more controversial stuff (I'm still a big fan of gcc's -Weffc++).
Would you agree that having a warning enabled that has uncovered one
real bug in N years of development, offers a bad cost/benefit ratio if
it needs recurring fixes in external code/frequently breaks HEAD?
Cheers,
--
Thorsten
If you're not failing some of the time, you're not trying hard enough.
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
