Re: [cmake-developers] Enabling more compiler warnings
On 11/06/2012 08:55 AM, Stephen Kelly wrote:
+ set(CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS} -Wnon-virtual-dtor -ansi -Wcast-
align -Wchar-subscripts -Wall -W -Wshadow)
Adding -ansi here will require several other fixes as
revealed by the dashboard last night.
Also, your fix commit:
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=853c22b2
added use of cmTarget::UNKNOWN_TARGET which does not exist.
It is UNKNOWN_LIBRARY.
Finally, existing dashboard scripts already add many of these
flags. Now we're duplicating them. They should be added in
a loop that adds each one only if not already present.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Enabling more compiler warnings
Brad King wrote:
On 11/06/2012 08:55 AM, Stephen Kelly wrote:
+ set(CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS} -Wnon-virtual-dtor -ansi
-Wcast- align -Wchar-subscripts -Wall -W -Wshadow)
Adding -ansi here will require several other fixes as
revealed by the dashboard last night.
I've removed that one. I'm not sure what it caused specifically. Was it all
of these? http://open.cdash.org/viewBuildError.php?buildid=2650259
Also, your fix commit:
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=853c22b2
added use of cmTarget::UNKNOWN_TARGET which does not exist.
It is UNKNOWN_LIBRARY.
Thanks, fixed now hopefully.
Finally, existing dashboard scripts already add many of these
flags. Now we're duplicating them. They should be added in
a loop that adds each one only if not already present.
I'll see if I can find a way to do that.
Does the cmake regex system have a way to match word boundaries? I need to
be able to match '-W\b' such that '-Wall' is not matched.
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Enabling more compiler warnings
On 11/07/2012 09:40 AM, Stephen Kelly wrote:
I've removed that one. I'm not sure what it caused specifically. Was it all
of these? http://open.cdash.org/viewBuildError.php?buildid=2650259
Yes.
Does the cmake regex system have a way to match word boundaries? I need to
be able to match '-W\b' such that '-Wall' is not matched.
No, but you can do
if(NOT ${CMAKE_C_FLAGS} MATCHES -W )
Note the space on the left side. That allows the right side
to match even at the beginning or end.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Enabling more compiler warnings
Brad King wrote:
On 11/07/2012 09:40 AM, Stephen Kelly wrote:
Does the cmake regex system have a way to match word boundaries? I need
to be able to match '-W\b' such that '-Wall' is not matched.
No, but you can do
if(NOT ${CMAKE_C_FLAGS} MATCHES -W )
Note the space on the left side. That allows the right side
to match even at the beginning or end.
Yes, but that won't work for the flags at the beginning and end.
Edge case (hehe) to live with or special cases for those with regexes with
^${flag} and ${flag}$? Even then ^${flag} won't match if there's
only one flag in there (because of the whitespace), so a full function to
check for existence gets complex to complete.
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Enabling more compiler warnings
On 11/07/2012 10:06 AM, Stephen Kelly wrote:
Brad King wrote:
On 11/07/2012 09:40 AM, Stephen Kelly wrote:
Does the cmake regex system have a way to match word boundaries? I need
to be able to match '-W\b' such that '-Wall' is not matched.
No, but you can do
if(NOT ${CMAKE_C_FLAGS} MATCHES -W )
Note the space on the left side. That allows the right side
to match even at the beginning or end.
Yes, but that won't work for the flags at the beginning and end.
Perhaps you should re-read my whole message more carefully ;)
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Enabling more compiler warnings
On 11/06/2012 08:55 AM, Stephen Kelly wrote:
If CMake enabled more warnings by default, developers would notice them
locally too before even submitting to the dashboard.
s/would/might/ but it could still be useful. I'd prefer extra warnings
only in a condition like
if(NOT CMake_VERSION_IS_RELEASE)
...
endif()
I would prefer to also add -Wundef, but kwsys is not clean with that flag
and creates many warnings, so it would have to be fixed there first.
I'm still catching up getting not-so-portable KWSys changes contributed
for another project into CMake. After that is done then we can look at it.
I'm fairly sure it would work with GCC 4.2 or later (and I guess clang as
well), which would probably cover many cmake developers. The GCC version
check only becomes a lot easier with a recent version of CMake
Just use
if(${CMAKE_C_COMPILER_ID} STREQUAL GNU AND
NOT ${CMAKE_C_COMPILER_VERSION} VERSION_LESS 4.2)
When built with a CMake that does not define the version variable
one simply won't get the extra warnings. Not a big deal.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Enabling more compiler warnings
On Tuesday 06 November 2012, Stephen Kelly wrote:
Hi there,
One common reason topcis warn on the dashboard and need to be updated and
then rebased is that some machines have more warnings enabled by others.
If CMake enabled more warnings by default, developers would notice them
locally too before even submitting to the dashboard. For example, many of
my follow up commits have been related to unused parameters, or order of
initialization in constructors.
I have a branch for enabling some warnings, but it would need to be
upstream to be of any use:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index ea1c033..3eb1a43 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -586,6 +586,19 @@ option(CMAKE_STRICT
mark_as_advanced(CMAKE_STRICT)
+if(${CMAKE_C_COMPILER_ID} MATCHES GNU)
+
+ set(CMAKE_C_FLAGS ${CMAKE_C_FLAGS} -Wcast-align -Werror-implicit-
function-declaration -Wchar-subscripts -Wall -W)
+ set(CMAKE_C_FLAGS ${CMAKE_C_FLAGS} -Wpointer-arith -Wwrite-strings -
Wformat-security -Wmissing-format-attribute -fno-common)
+ set(CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS} -Wnon-virtual-dtor -ansi -Wcast-
align -Wchar-subscripts -Wall -W -Wshadow)
+ set(CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS} -Wpointer-arith -Wformat-
security)
+
+ if (NOT APPLE)
+set(CMAKE_SHARED_LINKER_FLAGS -Wl,--fatal-warnings -Wl,--no-undefined
${CMAKE_SHARED_LINKER_FLAGS})
+set(CMAKE_MODULE_LINKER_FLAGS -Wl,--fatal-warnings -Wl,--no-undefined
${CMAKE_MODULE_LINKER_FLAGS})
+ endif ()
+endif()
+
I would prefer to also add -Wundef, but kwsys is not clean with that flag
and creates many warnings, so it would have to be fixed there first.
I'm fairly sure it would work with GCC 4.2 or later (and I guess clang as
well), which would probably cover many cmake developers. The GCC version
check only becomes a lot easier with a recent version of CMake (I don't
recall which), but I'm sure most people do cmake development using the
development branch anyway, but we'd have to wrap it in a CMAKE_VERSION
check anyway.
I'd prefer to just do it like this rather than with feature tests, because
my experience with feature tests on the CMake dashboard (with the export
header stuff a while back) indicates that it's not worth it in this case.
It would take far too long to get right.
Any thoughts?
+1
Alex
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
