[Insight-developers] A massive number of new warnings

Thu Jan 6 18:04:38 EST 2011

On the dashboards it's not really an issue because they report warnings or
errors, either one just fine.

The problem is sitting in front of the keyboard... If it's not an error that
prevents the build from completing, it will be ignored by *somebody*. And
then, it just sneaks right in and forces those of us who monitor the
dashboard health to take on the burden and the pain of cleaning it up. But
if it is an error, then that same somebody has to deal with it before moving
on: it'll get fixed much quicker if it's a build error for everybody who
sees it.

Of course, the other problem is that you can't build it yourself on *all*
the platforms and different warnings show up on different platforms. So, in
that respect, everybody causes a warning elsewhere once in a while through
no fault of their own.

But, just some food for thought: if "treat warnings as errors" were on by
default for everybody's build, and hard to turn off, how many warnings would
there be when compiling the source code. I'm guessing close to zero the vast
majority of the time.

;-)

On Thu, Jan 6, 2011 at 5:19 PM, Bill Lorensen <bill.lorensen at gmail.com>wrote:

> Eventually, if we can get warnings near zero, I agree that at least
> for our dashboards we should set the flag that treats warnings as
> error.
>
>
> On Thu, Jan 6, 2011 at 4:13 PM, David Cole <david.cole at kitware.com> wrote:
> > Actually the easiest way to get the point across (and win enemies and
> > influence people) is to set the flag that treats warnings as errors.
> >
> > That'll grab their attention!!
> >
> > (Of course, you might not be able to get past the Utilities directory
> > in the next decade, but.....)
> >
> >
> > :-)
> > David
> >
> >
> > On Thu, Jan 6, 2011 at 4:02 PM, Hans Johnson <hans-johnson at uiowa.edu>
> wrote:
> >> I was toying with the idea of adding something like is shown at the
> bottom
> >> of this message.
> >>
> >> We could safely build up a larger set of "recommended" compiler flags
> for
> >> each of the release types rather than just "-g" and "-O3".
> >>
> >> I think that -Wall should be the default flag, and that developers have
> to
> >> explicitly turn it off.  That way it's not just the 4 of us triaging all
> >> these complier warnings.
> >>
> >> NOTE:  The code below is not working, but I think it gets the point
> across.
> >>
> >> Hans
> >>
> >>
> >>
> >>
> >> #To create a portable build system, it is best to not test for
> platforms,
> >> but to test for features.
> >> #
> >> #Instead of testing "if Windows then do this", test for "if the
> >> -Wno-invalid-offsetof flag works then use it". You can do that with the
> >> CheckCCompilerFlag module, for example:
> >>
> >> include(CheckCCompilerFlag)
> >> include(CheckCXXCompilerFlag)
> >>
> >> set(C_RELEASE_DESIRED_FLAGS "")
> >> set(CXX_RELEASE_DESIRED_FLAGS "")
> >>
> >> # -Wall
> >> check_c_compiler_flag(-Wall HAS_ALL_WARNINGS_C)
> >> if (HAS_ALL_WARNINGS_C)
> >>    set(CMAKE_C_FLAGS  "${CMAKE_C_FLAGS} -Wall")
> >> endif()
> >>
> >> check_cxx_compiler_flag(-Wall HAS_ALL_WARNINGS_CXX)
> >> if (HAS_ALL_WARNINGS_CXX)
> >>    set(CMAKE_CXX_FLAGS  "${CMAKE_C_FLAGS} -Wall")
> >> endif()
> >>
> >> ## -pedantic
> >> check_c_compiler_flag(-pedantic HAS_PEDANTIC_C)
> >> if (HAS_PEDANTIC_C)
> >>    set(CMAKE_C_FLAGS  "${CMAKE_C_FLAGS} -pedantic")
> >> endif()
> >>
> >> check_cxx_compiler_flag(-pedantic HAS_PEDANTIC_CXX)
> >> if (HAS_PEDANTIC_CXX)
> >>    set(CMAKE_CXX_FLAGS  "${CMAKE_C_FLAGS} -pedantic")
> >> endif()
> >>
> >> ## -Wextra
> >> check_c_compiler_flag(-Wextra HAS_EXTRA_WARNINGS_C)
> >> if (HAS_EXTRA_WARNINGS_C)
> >>    set(CMAKE_C_FLAGS  "${CMAKE_C_FLAGS} -Wextra")
> >> endif()
> >>
> >> check_cxx_compiler_flag(-Wextra HAS_EXTRA_WARNINGS_CXX)
> >> if (HAS_EXTRA_WARNINGS_CXX)
> >>    set(CMAKE_CXX_FLAGS  "${CMAKE_C_FLAGS} -Wextra")
> >> endif()
> >>
> >> ## -Wshadow
> >> check_c_compiler_flag(-Wshadow HAS_SHADOW_WARNINGS_C)
> >> if (HAS_SHADOW_WARNINGS_C)
> >>    set(CMAKE_C_FLAGS  "${CMAKE_C_FLAGS} -Wshadow")
> >> endif()
> >>
> >> check_cxx_compiler_flag(-Wshadow HAS_SHADOW_WARNINGS_CXX)
> >> if (HAS_SHADOW_WARNINGS_CXX)
> >>    set(CMAKE_CXX_FLAGS "${CMAKE_C_FLAGS} -Wshadow")
> >> endif()
> >>
> >> ## -Wlong-long  We should not use the long-long type in ITK
> >> check_c_compiler_flag(-Wlong-long HAS_LONG_LONG_WARNINGS_C)
> >> if (HAS_LONG_LONG_WARNINGS_C)
> >>    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wlong-long")
> >> endif()
> >>
> >> check_cxx_compiler_flag(-Wlong-long HAS_LONG_LONG_WARNINGS_CXX)
> >> if (HAS_LONG_LONG_WARNINGS_CXX)
> >>    set(CMAKE_CXX_FLAGS "${CMAKE_C_FLAGS} -Wlong-long")
> >> endif()
> >>
> >>
> >> check_cxx_compiler_flag(-Wno-invalid-offsetof HAS_NO_INVALID_OFFSETOF)
> >> if (HAS_NO_INVALID_OFFSETOF)
> >>    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-invalid-offsetof")
> >> endif()
> >>
> >> message(${CMAKE_C_FLAGS})
> >> message(${CMAKE_CXX_FLAGS})
> >>
> >>
> >>
> >> On 1/6/11 2:55 PM, "Luis Ibanez" <luis.ibanez at kitware.com> wrote:
> >>
> >>> I just added -Wall and -Wextra to the builds in
> >>> zion, thurmite and eldorado.
> >>>
> >>> It's time to follow Bill L. in the zero-warning campaign      :-)
> >>>
> >>>
> >>>      Luis
> >>>
> >>>
> >>> ----------------------------------
> >>> On Wed, Jan 5, 2011 at 6:32 PM, Sean McBride <sean at rogue-research.com>
> wrote:
> >>>> That build is only using -Wall and -Wextra, which my gcc dashboards
> also
> >>>> use, and yet mine show a much smaller number of warnings.  Strange
> that
> >>>> a newer gcc would not warn where an older gcc does, one reason could
> be
> >>>> that they were false positives in the older version.  In any case,
> there
> >>>> are less warnings than it seems since some are caused by headers and
> so
> >>>> repeated in many source files.
> >>>>
> >>>> On Wed, 5 Jan 2011 17:01:46 -0500, Bill Lorensen said:
> >>>>
> >>>>> I think they are valid warnings.
> >>>>>
> >>>>> On Wed, Jan 5, 2011 at 4:20 PM, Hans Johnson <hans-johnson at uiowa.edu>
> wrote:
> >>>>>> This looks like the Insight-gcc-3.3. Has a definitive decision been
> made
> >>>>>> regarding compilers less than 4?  I thought that less than 3.4 was
> excluded
> >>>>>> because of incompatibility issues.
> >>>>
> >>>>
> >>>>
> >>
> >> --
> >> Hans J. Johnson, Ph.D.
> >> Assistant Professor
> >> 200 Hawkins Drive
> >> T205 BT, The University of Iowa
> >> Iowa City, IA 52242
> >>
> >> hans-johnson at uiowa.edu
> >> PHONE: 319 353 8587
> >>
> >> _______________________________________________
> >> Powered by www.kitware.com
> >>
> >> Visit other Kitware open-source projects at
> >> http://www.kitware.com/opensource/opensource.html
> >>
> >> Kitware offers ITK Training Courses, for more information visit:
> >> http://kitware.com/products/protraining.html
> >>
> >> Please keep messages on-topic and check the ITK FAQ at:
> >> http://www.itk.org/Wiki/ITK_FAQ
> >>
> >> Follow this link to subscribe/unsubscribe:
> >> http://www.itk.org/mailman/listinfo/insight-developers
> >>
> > _______________________________________________
> > Powered by www.kitware.com
> >
> > Visit other Kitware open-source projects at
> > http://www.kitware.com/opensource/opensource.html
> >
> > Kitware offers ITK Training Courses, for more information visit:
> > http://kitware.com/products/protraining.html
> >
> > Please keep messages on-topic and check the ITK FAQ at:
> > http://www.itk.org/Wiki/ITK_FAQ
> >
> > Follow this link to subscribe/unsubscribe:
> > http://www.itk.org/mailman/listinfo/insight-developers
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20110106/5bb9a652/attachment.htm>