I have been thinking about compiler warnings because of the recent discussion here: http://www.mail-archive.com/lyx-devel@lists.lyx.org/msg175099.html
>From that thread, I have the understanding that some developers think it's important to fix warnings, although not if it is risky. I don't have much experience programming so I wouldn't be surprised if there's something wrong with the following logic: It seems to me that warnings should either be fixed when they occur or just ignored. That is, if a warning is viewed to be a problem, it should be fixed right when it's introduced. Or, if it is not viewed as a problem, it should just be permanently ignored. I don't see any advantage to having warnings sit around. Thus, I wonder if it would be useful to specify "-Werror" in the development build, which would turn all warnings into errors and would alert the author to fix them right away. The current patch does this for autotools and cmake. I think this would be good because (1) who better to fix a warning than the author of the code and (2) when better to fix a warning than when the code is fresh in the memory of the author? I also think that (3) because of (1) and (2) the risk of fixing a warning would be minimized. I'm guessing that there are some warnings that should not be addressed? If so, exceptions can be added to treat them as warnings or ignore them. For example, in trunk there has been the following warning introduced a while ago. filetools.cpp: In function ‘bool lyx::support::readLink(const lyx::support::FileName&, lyx::support::FileName&)’: filetools.cpp:849:28: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] For some reason it only shows up with the current autotools setup when -Werror is specified and does not show up when it is omitted or under the current cmake setup. I don't understand why this is. In any case, if this specific error should be a warning, we can do the following: #pragma GCC diagnostic warning "-Wsign-compare" if (nRead < buf.size() - 1) { break; } #pragma GCC diagnostic error "-Wsign-compare" However, I have trouble thinking of a case where one would want a warning sitting around. If it's a legitimate warning that should be addressed, it should be an error and should be fixed. If it's a false alarm, shouldn't it just be permanently ignored? If so, "warning" can be replaced with "ignored" in the above pragmas and no message will appear. Comments? Thanks, Scott
From 33f38e72f5fcb1e60dab1ed28ca463473fc0a526 Mon Sep 17 00:00:00 2001 From: Scott Kostyshak <skost...@lyx.org> Date: Sat, 29 Sep 2012 04:30:28 -0400 Subject: [PATCH] Convert compiler warnings into errors Add the -Werror flag for both autotools and cmake. If a warning should not be converted into an error, an exception can be specified by using diagnostic pragmas. --- CMakeLists.txt | 2 +- config/lyxinclude.m4 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f04d6cf..dc9d12b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -412,7 +412,7 @@ if(NOT MSVC) if(NOT LYX_QUIET) set(CMAKE_VERBOSE_MAKEFILE ON) endif() - set(LYX_CXX_FLAGS "-Wall -Wunused-parameter") + set(LYX_CXX_FLAGS "-Wall -Wunused-parameter -Werror") if(LYX_STDLIB_DEBUG) set(LYX_CXX_FLAGS "${LYX_CXX_FLAGS} -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC") endif() diff --git a/config/lyxinclude.m4 b/config/lyxinclude.m4 index d915d5c..d8c0be9 100644 --- a/config/lyxinclude.m4 +++ b/config/lyxinclude.m4 @@ -270,7 +270,7 @@ if test x$GXX = xyes; then CPPFLAGS="-W -Wall $CPPFLAGS" ;; *) - CPPFLAGS="-Wextra -Wall $CPPFLAGS " + CPPFLAGS="-Wextra -Wall -Werror $CPPFLAGS " ;; esac fi -- 1.7.9.5