Thorsten Behrens wrote:
Stephan Bergmann <[EMAIL PROTECTED]> writes:

Thorsten Behrens wrote:
[...]
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?

Do we ever want to upgrade the STLport versions we use? I doubt that: On the one hand, we are tied to compatibility issues as STLport details are exposed in the UNO/URE API (though I do not know whether upgrading to later STLport versions would actually be a compatibility issue); on the other hand, if we ever do upgrade to something more modern, I would assume we dump STLport and use the standard libraries that come directly with the compilers. In light of this, neither do I think it would make sense to try to upstream the patches we did to STLport, nor do I see any scalability issues.

[...]
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?

I agree that if we plan to upgrade boost in the future, and it appears unlikely that future boost versions will no longer trigger C4668, then it might make sense to disable C4668 globally instead of locally within a boost patch.

-Stephan

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to