Re: [Libreoffice] --enable-werror

On Mon, 2011-09-26 at 11:40 +0100, Michael Meeks wrote:
> We had a situation for several days with master giving tens of
>  thousands of warnings while building.

--enable-werror is a tricky thing alright given the different compilers,
even within the gcc family, or maybe especially within the gcc family,
so --enable-werror can't practically be proscriptive or defaulted on.

Best thing IMO to do is to give --enable-werror a shot, e.g. now with
master, and see if it works for you. If it does work, then you're in the
happy-compiler gang, and you can keep it enabled without a great deal of
misery. For me, on 4.6.0, there are 0 warnings in any of our source.

Plenty of previous gcc releases have been a bit confused with warnings
inside anonymous namespaces, and baffling visibility warnings which go
away when you try to make reduced test-cases. 4.6.X is a "good" compiler
for me, some of the earlier 4.X.Y compilers weren't. I try and keep an
eye out for what's coming down the track in fedora rawhide gcc and patch
us early, or get some test-cases back for the compiler people, though I
haven't got around to this for 4.7.X yet.

One of the most common issues is that 4.6.X(?) can tell you about
assigned but unused variables, while older ones can't. I tend to be
quite happy when the build breaks for me on those, because there's a
good chance there's some code which can be removed. One of the common
windows warnings, is a little bogus about unreachable code, but can also
lead to the simplification of various methods. But only appears I think
in msvc 2008 express.

C.

FWIW, the week-end warnings issue was around this chain of events over
the last few months...

a) SAL_N_ELEMENTS(arr) is a convenient "get number of elements in array"
macro

b) I saw the opportunity a while ago to use a c++ template for cplusplus
code to catch at compile-time any misuses of SAL_N_ELEMENTS on a
pointer, e.g. 

#define SAL_N_ELEMENTS(arr) (sizeof (arr) / sizeof ((arr)[0]))

int main(void)
{
    const char hello_world_a[] = "hello world";
    const char *hello_world_b = "hello world";

    printf("%d\n", SAL_N_ELEMENTS("hello world")); //ok
    printf("%d\n", SAL_N_ELEMENTS(hello_world_a)); //ok
    printf("%d\n", SAL_N_ELEMENTS(hello_world_b)); //not what you wanted
    return 0;
}

compiles fine, gives...
12
12
8 <-- epic fail (64bit sizeof(char*))

but...

template <typename T, size_t S> char (&sal_n_array_size( T(&)[S] ))[S];
#define SAL_N_ELEMENTS(arr) (sizeof(sal_n_array_size(arr)))

int main(void)
{
    const char hello_world_a[] = "hello world";
    const char *hello_world_b = "hello world";

    printf("%d\n", SAL_N_ELEMENTS("hello world"));
    printf("%d\n", SAL_N_ELEMENTS(hello_world_a));
    printf("%d\n", SAL_N_ELEMENTS(hello_world_b));
    return 0;
}

test.cxx: In function ‘int main()’:
test.cxx:14:20: error: no matching function for call to
‘sal_n_array_size(const char*&)’
test.cxx:14:20: note: candidate is:
test.cxx:4:79: note: template<class T, long unsigned int S> char (&
sal_n_array_size(T (&)[S]))[S]

aha, gotcha at compile time, awesome, so RTL_CONSTASCII_USTRINGPARAM and
RTL_CONSTASCII_STRINGPARAM can be defined in terms of SAL_N_ELEMENTS and
that allowed us to fine a pile of bugs with the misuse of those too.

c) catch though, is that you need this language defect,
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#757 or
something of that nature, fixed in order to use the template on certain
structs for various boring reasons, so when we can do it we build with
-std=c++0x where this is fixed (configure test)

d) catch with *that* though is that std::auto_ptr is deprecated in 
c++11, so... I originally passed the don't deprecated global option to 
g++ so there wouldn't be masses of warning spew, and --enable-werror
would remain useful.

e) catch with *that* though, is that Bjorne's deprecation macro then
wouldn't do anything, so I went back and used pragma push/pop on the
warnings around all the auto_ptr's I couldn't trivially replace with
boost::scoped_ptr so we could get close to the best of both worlds.

f) As it turned out, gcc 4.5.X >= can do --std=c++0x, but you need gcc
4.6.X >= for the pragma push/pop. i.e. there was a window of 4.5.X users
who got the mass of warning spew on deprecated auto_ptr's for a little
while, its back to disabling deprecation globally for 4.5.X guys.