From e4b6487fcaec5c5212aec237d764fc9e8d28f1d4 Mon Sep 17 00:00:00 2001 From: Ian McInerney Date: Wed, 6 May 2020 01:47:20 +0100 Subject: [PATCH] Overhaul compiler warnings infrastructure * Track our warnings separate from normal flags * Remove all warnings from the SWIG code * Add more GCC warnings --- CMakeLists.txt | 5 --- CMakeModules/Warnings.cmake | 55 ++++++++++++++++++++----- bitmap2component/CMakeLists.txt | 7 ++-- common/CMakeLists.txt | 7 ++-- cvpcb/CMakeLists.txt | 7 ++-- eeschema/CMakeLists.txt | 7 ++-- gerbview/CMakeLists.txt | 7 ++-- kicad/CMakeLists.txt | 7 ++-- pagelayout_editor/CMakeLists.txt | 7 ++-- pcbnew/CMakeLists.txt | 29 +++++-------- plugins/CMakeLists.txt | 7 ++-- thirdparty/markdown2html/CMakeLists.txt | 7 ++-- utils/CMakeLists.txt | 7 ++-- 13 files changed, 95 insertions(+), 64 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a288395aac..0c93ad6d61 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -302,11 +302,6 @@ if( CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) set( CMAKE_CXX_FLAGS "-ffloat-store ${CMAKE_CXX_FLAGS}" ) endif() - # Establish -Wall early, so specialized relaxations of this may come - # subsequently on the command line, such as in pcbnew/github/CMakeLists.txt - set( CMAKE_C_FLAGS "-Wall ${CMAKE_C_FLAGS}" ) - set( CMAKE_CXX_FLAGS "-Wall ${CMAKE_CXX_FLAGS}" ) - # Link flags in Debug: -g1 and -ggdb1 or -g1 and -ggdb3 can be used. # Level 1 produces minimal information, enough for making basic backtraces. # This includes descriptions of functions and external variables, and line number tables, diff --git a/CMakeModules/Warnings.cmake b/CMakeModules/Warnings.cmake index a0f6084187..bd369571d6 100644 --- a/CMakeModules/Warnings.cmake +++ b/CMakeModules/Warnings.cmake @@ -26,11 +26,23 @@ if( CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) include( CheckCXXCompilerFlag ) + # The SWIG-generated files tend to throw a lot of warnings, so + # we do not add the warnings directly to the flags here but instead + # keep track of them and add them to the flags later in a controlled manner + # (that way we can not put any warnings on the SWIG-generated files) + set( COMPILER_SUPPORTS_WARNINGS TRUE ) + + # Establish -Wall early, so specialized relaxations of this may come + # subsequently on the command line, such as in pcbnew/github/CMakeLists.txt + set( WARN_FLAGS_C "-Wall" ) + set( WARN_FLAGS_CXX "-Wall" ) + + # Warn about missing override specifiers CHECK_CXX_COMPILER_FLAG( "-Wsuggest-override" COMPILER_SUPPORTS_WSUGGEST_OVERRIDE ) if( COMPILER_SUPPORTS_WSUGGEST_OVERRIDE ) - set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wsuggest-override" ) + set( WARN_FLAGS_CXX "${WARN_FLAGS_CXX} -Wsuggest-override" ) message( STATUS "Enabling warning -Wsuggest-override" ) endif() @@ -39,15 +51,34 @@ if( CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) CHECK_CXX_COMPILER_FLAG( "-Winconsistent-missing-override" COMPILER_SUPPORTS_WINCONSISTENT_MISSING_OVERRIDE ) if( COMPILER_SUPPORTS_WINCONSISTENT_MISSING_OVERRIDE ) - set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Winconsistent-missing-override" ) + set( WARN_FLAGS_CXX "${WARN_FLAGS_CXX} -Winconsistent-missing-override" ) message( STATUS "Enabling warning -Winconsistent-missing-override" ) endif() + # Warn on duplicated branches + CHECK_CXX_COMPILER_FLAG( "-Wduplicated-branches" COMPILER_SUPPORTS_WDUPLICATED_BRANCHES ) + + if( COMPILER_SUPPORTS_WDUPLICATED_BRANCHES ) + set( WARN_FLAGS_CXX "${WARN_FLAGS_CXX} -Wduplicated-branches" ) + message( STATUS "Enabling warning -Wduplicated-branches" ) + endif() + + + # Warn on duplicated conditions + CHECK_CXX_COMPILER_FLAG( "-Wduplicated-cond" COMPILER_SUPPORTS_WDUPLICATED_COND ) + + if( COMPILER_SUPPORTS_WDUPLICATED_COND ) + set( WARN_FLAGS_CXX "${WARN_FLAGS_CXX} -Wduplicated-cond" ) + message( STATUS "Enabling warning -Wduplicated-cond" ) + endif() + + + CHECK_CXX_COMPILER_FLAG( "-Wvla" COMPILER_SUPPORTS_WVLA ) if( COMPILER_SUPPORTS_WVLA ) - set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror=vla" ) + set( WARN_FLAGS_CXX "${WARN_FLAGS_CXX} -Werror=vla" ) message( STATUS "Enabling error for -Wvla" ) endif() @@ -56,7 +87,13 @@ if( CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) CHECK_CXX_COMPILER_FLAG( "-Wimplicit-fallthrough" COMPILER_SUPPORTS_WIMPLICIT_FALLTHROUGH ) if( COMPILER_SUPPORTS_WIMPLICIT_FALLTHROUGH ) - set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wimplicit-fallthrough" ) + if( CMAKE_COMPILER_IS_GNUCXX ) + # GCC level 5 does not allow comments - mirrors the Clang warning + set( WARN_FLAGS_CXX "${WARN_FLAGS_CXX} -Wimplicit-fallthrough=5" ) + else() + set( WARN_FLAGS_CXX "${WARN_FLAGS_CXX} -Wimplicit-fallthrough" ) + endif() + message( STATUS "Enabling warning -Wimplicit-fallthrough" ) endif() @@ -65,20 +102,16 @@ if( CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) CHECK_CXX_COMPILER_FLAG( "-Wreturn-type" COMPILER_SUPPORTS_WRETURN_TYPE ) if( COMPILER_SUPPORTS_WRETURN_TYPE ) - set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror=return-type" ) + set( WARN_FLAGS_CXX "${WARN_FLAGS_CXX} -Werror=return-type" ) message( STATUS "Enabling error for -Wreturn-type" ) endif() - # Warn about shadowed variables (-Wshadow option), if supported - # Unfortunately, the swig autogenerated files have a lot of shadowed variables - # and -Wno-shadow does not exist. - # Adding -Wshadow can be made only for .cpp files - # and will be added later in CMakeLists.txt + # Warn about shadowed variables CHECK_CXX_COMPILER_FLAG( "-Wshadow" COMPILER_SUPPORTS_WSHADOW ) if( COMPILER_SUPPORTS_WSHADOW ) - set( WSHADOW_FLAGS "-Wshadow" ) + set( WARN_FLAGS_CXX "${WARN_FLAGS_CXX} -Wshadow" ) message( STATUS "Enabling warning -Wshadow" ) endif() diff --git a/bitmap2component/CMakeLists.txt b/bitmap2component/CMakeLists.txt index 86c42ec29b..766c303f07 100644 --- a/bitmap2component/CMakeLists.txt +++ b/bitmap2component/CMakeLists.txt @@ -1,6 +1,7 @@ -# .cpp files are compiled with extra ${WSHADOW_FLAGS} -if( COMPILER_SUPPORTS_WSHADOW ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WSHADOW_FLAGS}") +# Add all the warnings to the files +if( COMPILER_SUPPORTS_WARNINGS ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WARN_FLAGS_CXX}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARN_FLAGS_C}") endif() add_definitions( -DBITMAP_2_CMP ) diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index 704def78cc..8daf045f28 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -1,6 +1,7 @@ -# .cpp files are compiled with extra ${WSHADOW_FLAGS} -if( COMPILER_SUPPORTS_WSHADOW ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WSHADOW_FLAGS}") +# Add all the warnings to the files +if( COMPILER_SUPPORTS_WARNINGS ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WARN_FLAGS_CXX}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARN_FLAGS_C}") endif() add_subdirectory( libeval ) diff --git a/cvpcb/CMakeLists.txt b/cvpcb/CMakeLists.txt index 457c81ddf0..6755cdfad7 100644 --- a/cvpcb/CMakeLists.txt +++ b/cvpcb/CMakeLists.txt @@ -1,6 +1,7 @@ -# .cpp files are compiled with extra ${WSHADOW_FLAGS} -if( COMPILER_SUPPORTS_WSHADOW ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WSHADOW_FLAGS}") +# Add all the warnings to the files +if( COMPILER_SUPPORTS_WARNINGS ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WARN_FLAGS_CXX}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARN_FLAGS_C}") endif() add_definitions( -DCVPCB ) diff --git a/eeschema/CMakeLists.txt b/eeschema/CMakeLists.txt index 3548114718..1d3c7e2150 100644 --- a/eeschema/CMakeLists.txt +++ b/eeschema/CMakeLists.txt @@ -1,6 +1,7 @@ -# .cpp files are compiled with extra ${WSHADOW_FLAGS} -if( COMPILER_SUPPORTS_WSHADOW ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WSHADOW_FLAGS}") +# Add all the warnings to the files +if( COMPILER_SUPPORTS_WARNINGS ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WARN_FLAGS_CXX}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARN_FLAGS_C}") endif() add_definitions( -DEESCHEMA ) diff --git a/gerbview/CMakeLists.txt b/gerbview/CMakeLists.txt index 9f9b6d765b..24fd6d110d 100644 --- a/gerbview/CMakeLists.txt +++ b/gerbview/CMakeLists.txt @@ -1,6 +1,7 @@ -# .cpp files are compiled with extra ${WSHADOW_FLAGS} -if( COMPILER_SUPPORTS_WSHADOW ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WSHADOW_FLAGS}") +# Add all the warnings to the files +if( COMPILER_SUPPORTS_WARNINGS ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WARN_FLAGS_CXX}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARN_FLAGS_C}") endif() add_definitions(-DGERBVIEW) diff --git a/kicad/CMakeLists.txt b/kicad/CMakeLists.txt index a54c1f5919..dccd99739f 100644 --- a/kicad/CMakeLists.txt +++ b/kicad/CMakeLists.txt @@ -1,6 +1,7 @@ -# .cpp files are compiled with extra ${WSHADOW_FLAGS} -if( COMPILER_SUPPORTS_WSHADOW ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WSHADOW_FLAGS}") +# Add all the warnings to the files +if( COMPILER_SUPPORTS_WARNINGS ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WARN_FLAGS_CXX}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARN_FLAGS_C}") endif() add_definitions( -DKICAD ) diff --git a/pagelayout_editor/CMakeLists.txt b/pagelayout_editor/CMakeLists.txt index 79b3b9d7a0..b4ea9e52d5 100644 --- a/pagelayout_editor/CMakeLists.txt +++ b/pagelayout_editor/CMakeLists.txt @@ -1,6 +1,7 @@ -# .cpp files are compiled with extra ${WSHADOW_FLAGS} -if( COMPILER_SUPPORTS_WSHADOW ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WSHADOW_FLAGS}") +# Add all the warnings to the files +if( COMPILER_SUPPORTS_WARNINGS ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WARN_FLAGS_CXX}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARN_FLAGS_C}") endif() add_definitions(-DPL_EDITOR) diff --git a/pcbnew/CMakeLists.txt b/pcbnew/CMakeLists.txt index 80adea70a6..8935fabd2f 100644 --- a/pcbnew/CMakeLists.txt +++ b/pcbnew/CMakeLists.txt @@ -359,36 +359,29 @@ set( PCBNEW_SCRIPTING_PYTHON_HELPERS swig/python_scripting.cpp ) -if( COMPILER_SUPPORTS_WSHADOW ) - # .cpp files are compiled with extra ${WSHADOW_FLAGS}, but not .cxx files + +if( COMPILER_SUPPORTS_WARNINGS ) + # Only compile our source files with the warnings, since the SWIG generated + # files contain a lot of warnings, we just ignore it. set_source_files_properties( ${PCBNEW_SRCS} ${PCBNEW_COMMON_SRCS} ${PCBNEW_SCRIPTING_DIALOGS} ${PCBNEW_SCRIPTING_PYTHON_HELPERS} - PROPERTIES COMPILE_FLAGS ${WSHADOW_FLAGS} - ) - - # There is a lot of 'local variable shadowed' warnings, - # but we have no control over the generated code - set_source_files_properties( pcbnew_wrap.cxx pcbnewPYTHON_wrap.cxx - PROPERTIES COMPILE_FLAGS -Wno-shadow + PROPERTIES COMPILE_FLAGS ${WARN_FLAGS_CXX} ) endif() if( KICAD_SCRIPTING ) + + # Disable all warnings for the SWIG file + if( CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) + set_source_files_properties( pcbnew_wrap.cxx PROPERTIES COMPILE_FLAGS "-Wno-everything" ) + endif() + set( PCBNEW_SCRIPTING_SRCS ${PCBNEW_SCRIPTING_DIALOGS} pcbnew_wrap.cxx ${PCBNEW_SCRIPTING_PYTHON_HELPERS} ) - - - # Swig generated files do not use the override specifier, therefore - # disable suggest-override warnings - if( COMPILER_SUPPORTS_WSUGGEST_OVERRIDE ) - set_source_files_properties( pcbnew_wrap.cxx pcbnewPYTHON_wrap.cxx - PROPERTIES COMPILE_FLAGS -Wno-suggest-override - ) - endif() endif() diff --git a/plugins/CMakeLists.txt b/plugins/CMakeLists.txt index 720348efc5..9f8782aaf8 100644 --- a/plugins/CMakeLists.txt +++ b/plugins/CMakeLists.txt @@ -1,6 +1,7 @@ -# .cpp files are compiled with extra ${WSHADOW_FLAGS} -if( COMPILER_SUPPORTS_WSHADOW ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WSHADOW_FLAGS}") +# Add all the warnings to the files +if( COMPILER_SUPPORTS_WARNINGS ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WARN_FLAGS_CXX}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARN_FLAGS_C}") endif() include_directories( diff --git a/thirdparty/markdown2html/CMakeLists.txt b/thirdparty/markdown2html/CMakeLists.txt index e8fe7fc540..bd5010dc1f 100644 --- a/thirdparty/markdown2html/CMakeLists.txt +++ b/thirdparty/markdown2html/CMakeLists.txt @@ -1,6 +1,7 @@ -# .cpp files are compiled with extra ${WSHADOW_FLAGS} -if( COMPILER_SUPPORTS_WSHADOW ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WSHADOW_FLAGS}") +# Add all the warnings to the files +if( COMPILER_SUPPORTS_WARNINGS ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WARN_FLAGS_CXX}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARN_FLAGS_C}") endif() include_directories( BEFORE ${INC_BEFORE} ${INC_AFTER} ) diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index 78ad79863d..597e5a9a47 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -1,6 +1,7 @@ -# .cpp files are compiled with extra ${WSHADOW_FLAGS} -if( COMPILER_SUPPORTS_WSHADOW ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WSHADOW_FLAGS}") +# Add all the warnings to the files +if( COMPILER_SUPPORTS_WARNINGS ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${WARN_FLAGS_CXX}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARN_FLAGS_C}") endif() add_subdirectory( idftools )