Re[2]: [lmi] A matter of style

[Vadim Zeitlin]

On Wed, 07 Mar 2007 03:37:32 +0000 Greg Chicares <address@hidden> wrote:

 Sorry for not answering this before but I wanted to test this myself
before replying, but as I never found time to do it (as it didn't seem very
important) I'd like to at least write my proposal about how to deal with
this issue.

 I'm (exceptionally) keeping the entire message I'm replying to to preserve
the context:

GC> On 2007-3-7 0:58 UTC, Vadim Zeitlin wrote:
GC> [...]
GC> >  So I'd like to recommend to try to enable -Wunreachable-code for LMI
GC> > compilation and then allow writing just
GC> > 
GC> >   if ( condition )
GC> >           return;
GC> > 
GC> > as forgetting to add the braces will result in a warning.
GC> 
GC> That's an intriguing idea.
GC> 
GC> From time to time, I try rebuilding everything from scratch with
GC> stronger warnings. I don't often find that I can reenable a
GC> suppressed warning, but sometimes I do find errors in lmi itself
GC> that had been suppressed because of diagnostics arising from
GC> libraries we use. It was time to do that again anyway, so thanks
GC> for prompting me.
GC> 
GC> This particular warning is documented in 'workhorse.make':
GC>   # Too many warnings for libstdc++:
GC>   #  -Wunreachable-code \
GC> 
GC> I rebuilt with that warning and filtered the output this way:
GC>   sed -e'/executed/!d' | sort | uniq
GC> to remove duplicates because there were over nine hundred
GC> warnings, of which the thirty-two pasted below [1] are unique.
GC> Of those, seventeen are in libstdc++.
GC> 
GC> We could consider inhibiting this default:
GC>   -Wsystem-headers
GC>     Print warning messages for constructs found in system header files.
GC> assuming that a '-Wno-system-headers' form is defined. But that
GC> would still leave five unique warnings in third-party libraries,
GC> the one in 'boost/shared_ptr.hpp' being the worst because it
GC> affects much of our own code.

 I'm not sure why do you propose to use -Wsystem-headers (which is the
default anyhow and using -Wno-system-headers wouldn't help at all) but I do
think we could use -isystem instead of -I for the boost directories. Again,
I still didn't test this myself but it really _should_ work. And
-Wunreachable-code is a pretty useful warning, independently of the style
issue discussed in the original thread.

GC> I have a strong prejudice in favor of keeping
GC>   -pedantic-errors -Werror
GC> to treat all warnings as errors. If we weaken that, then every
GC> time we compile we'll have to filter out diagnostics that we've
GC> deemed benign. If we did that manually, I'm sure we'd overlook
GC> some actual problems, and the code would suffer. We could write
GC> a script to do that, but then that script would have to be
GC> maintained carefully, every time we update the compiler or a
GC> library. I don't see this as a good path to follow.

 No, I definitely agree that -Werror should be used and adding an extra
script is undesirable. I just think we should have no troubles at all not
generating the warnings for the standard headers in the first place...

GC> As always, I welcome your thoughts. I find that if I expose
GC> my reasoning in detail, you sometimes point out premises
GC> that I should change, resulting in a better outcome.
GC> 
GC> ---------
GC> 
GC> [1] Unique '-Wunreachable-code' warnings:
GC> 
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/allocator.h:103:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/basic_ios.h:268:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/basic_string.h:1988:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/basic_string.h:418:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/basic_string.h:426:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/stl_algo.h:208:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/stl_algobase.h:380:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/stl_iterator.h:666:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/stl_iterator.h:700:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/stl_tree.h:180:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/stl_vector.h:106:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/stl_vector.h:95:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/vector.tcc:257:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bits/vector.tcc:260:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/bitset:1019:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/new:56:
 warning: will never be executed
GC> 
C:/MinGW-20050827//bin/../lib/gcc/mingw32/3.4.4/../../../../include/c++/3.4.4/new:96:
 warning: will never be executed
GC> C:/lmi/src/lmi/actuarial_table.cpp:263: warning: will never be executed
GC> C:/lmi/src/lmi/actuarial_table.cpp:264: warning: will never be executed
GC> C:/lmi/src/lmi/actuarial_table.cpp:265: warning: will never be executed
GC> C:/lmi/src/lmi/actuarial_table.cpp:275: warning: will never be executed
GC> C:/lmi/src/lmi/crc32.cpp:147: warning: will never be executed
GC> C:/lmi/src/lmi/ihs_fpios.cpp:177: warning: will never be executed
GC> C:/lmi/src/lmi/ihs_pios.cpp:142: warning: will never be executed
GC> C:/lmi/src/lmi/mvc_model.cpp:67: warning: will never be executed
GC> C:/lmi/src/lmi/obstruct_slicing.hpp:99: warning: will never be executed
GC> C:/lmi/src/lmi/xenumtypes.cpp:979: warning: will never be executed
GC> C:/opt/lmi/third_party/include/boost/shared_ptr.hpp:252: warning: will 
never be executed
GC> C:/opt/lmi/third_party/src/cgicc/CgiUtils.cpp:68: warning: will never be 
executed
GC> C:/opt/lmi/third_party/src/libxml/ait_impl.cxx:273: warning: will never be 
executed
GC> C:/opt/lmi/third_party/src/libxml/node.cxx:113: warning: will never be 
executed
GC> C:/opt/lmi/third_party/src/libxml/tree_parser.cxx:190: warning: will never 
be executed

 I find it very strange that we get warnings from the standard g++ include
directories, apparently something is wrong with mingw32 gcc configuration
if it doesn't apply the system directory treatment to them. But maybe
things are better under Cygwin which you use now? Also, maybe explicitly
using -isystem with the libstdc++ directories could help too? At the very
least I don't get any such warnings under Linux (which uses the same files
and mostly the same compiler) which would seem that all that is missing is
making gcc understand that this directory is a system one.

 Regards,
VZ