[Insight-developers] Change in ITK[master]: COMP: Removed C++0x construct to silence warning

[Bradley Lowekamp]

Hans,

On Dec 23, 2010, at 9:45 AM, Hans Johnson wrote:

> Brad,
> 
> 1) Adding the global warnings to errors flag would cause many difficulties
> (See item 3 for alternative option).  Clang is finding a lot of minor (real,
> but minor) coding lapses.  For instance unused variables would now cause
> compiler failures.  This is problematic for the Utilities directory where it
> takes a long time to remove those warnings.

I agree global warnings into error is a bad idea.

> 
> 2)  There is a flag in clang for allowing support of C++0x, and we can turn
> that on to remove the warning.  I just thought it is interesting that we
> have been taking advantage of C++0x constructs for many years without even
> noticing it!
> 

There are many compiler extensions that we are using across the toolkit. I don't think anyone has tried to use any of the strict standard compilation flags.

> 3) This was my quick attempt at removing 1000's of warnings.  After reading
> your recommendation, I do think that a targeted clang compiler modification
> in the main ITK CMakeLists.txt file would work (Similar to the
> template-depth flags for the gcc compiler).  There are several ways we can
> go:  a)  silence this warning, and compile the code with this feature used,
> b) turn just this warning into an error at configuration (it will pass
> because the Cmake try/compile will fail).

This is a an interesting case because we are running a try compile, to determine if the compiler has this extensions. As the try_compile passes, we utilize the compiler extension. But if we don't want to show any warnings during the build, then if this try_compile should fail. 

However, I believe that this compiler feature produces better code, and potentially better run-time performance, so I am not sure the extension should be disabled just for this warning. I think I am of the opinion, that this is really not a useful warning and should not be enabled of the dashboard machine.

Brad


> 
> Please let me know what you think is the best option.
> 
> Hans
> 
> 
> 
> On 12/23/10 8:12 AM, "Arnaud Gelas (Code Review)"
> <gerrit2 at public.kitware.com> wrote:
> 
>> Bradley Lowekamp has posted comments on this change.
>> 
>> Change subject: COMP:  Removed C++0x construct to silence warning
>> ......................................................................
>> 
>> 
>> Patch Set 1:
>> 
>> I am not sure what I think of this change. The compiler is able to build this
>> code in the try_compile, and the compiler option is specifically asking for
>> warning like this. The in-class initialization can be superior to the external
>> initialization due to possible inlining optimizations. So we have a bit of a
>> mixed bag.
>> 
>> Is there a CMake, or clang flag to treat compiler warnings as errors? Adding
>> that to this try compile may be the best solution.
>> 
>> --
>> To view, visit http://review.source.kitware.com/611
>> To unsubscribe, visit http://review.source.kitware.com/settings
>> 
>> Gerrit-MessageType: comment
>> Gerrit-Change-Id: I5fd89944929990359b8f4979fc8833d22edc67f6
>> Gerrit-PatchSet: 1
>> Gerrit-Project: ITK
>> Gerrit-Branch: master
>> Gerrit-Owner: Hans J. Johnson <hans-johnson at uiowa.edu>
>> Gerrit-Reviewer: Bradley Lowekamp <blowekamp at mail.nih.gov>
> 
> -- 
> 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
> 

========================================================
Bradley Lowekamp  
Lockheed Martin Contractor for
Office of High Performance Computing and Communications
National Library of Medicine 
blowekamp at mail.nih.gov


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