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

Reply via email to