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::min<sal_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]
