[Insight-developers] A massive number of new warnings

Thu Jan 6 21:11:44 EST 2011

Hello,

I have a hard time getting excited about this issues when logic and valid code, which may be more optimal needs to be changes to suppress a warning only when -Wextra is enabled:

http://review.source.kitware.com/#change,677

There are useful warning and there are un-useful ones. At any rate, I agree that the dashboard needs some work to clean up some of these warnings. And I applaud the effort, and wish for more snow in New York to help get it done :)

Brad

On Jan 6, 2011, at 6:04 PM, David Cole wrote:

> 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
> >
> 
> <ATT00001..txt>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20110106/4a3e4f54/attachment.htm>