Aggressive compilation warnings (#2304)

CMakeLists.txt

@@ -23,22 +23,20 @@ option(BUILD_LIBMAMBAPY "Build libmamba Python bindings" OFF)
 option(BUILD_LIBMAMBA_TESTS "Build libmamba C++ tests" OFF)
 option(BUILD_MICROMAMBA "Build micromamba" OFF)
 option(BUILD_MAMBA_PACKAGE "Build mamba package utility" OFF)
+option(MAMBA_WARNING_AS_ERROR "Treat compiler warnings as errors" OFF)
+
+include("cmake/CompilerWarnings.cmake")
 
 if (MSVC)
     # NOMINMAX : prevent tons of code to be included when having to `#include <windows.h>`
     # /EHsc : enable C++ exceptions (otherwise exceptions do not work)
     # /Zc:__cplusplus : makes sure `__cplusplus` is set to the current C++ version language. Otherwise it is always set to an incorrect wrong value.
     # /MP : enable multi-process build with MSBuild (it should be on by default but CMake generates the project files without the right params).
-    # /W4 : warnings level 4 (default in visual studio projects and recommended minimum level).
     # /external:I $ENV{CONDA_PREFIX}: consider the conda env prefix libraries headers as "external" to this project.
-    # /external:W0 : set the warning level to 1 for external libraries headers (severe warnings, default when unspecified) to only see important warnings coming from dependencies headers.
-    string(REGEX REPLACE "/W[0-9]" "/W4" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /D_CRT_SECURE_NO_WARNINGS /DNOMINMAX /EHsc /Zc:__cplusplus /MP /experimental:external /external:I $ENV{CONDA_PREFIX} /external:W1")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /D_CRT_SECURE_NO_WARNINGS /DNOMINMAX /EHsc /Zc:__cplusplus /MP /experimental:external /external:I $ENV{CONDA_PREFIX}")
     # Force release mode to avoid debug libraries to be linked
     set(CMAKE_BUILD_TYPE Release)
     # add_definitions("-DUNICODE -D_UNICODE")
-else()
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall")
 endif()
 
 # Variants

cmake/CompilerWarnings.cmake

@@ -0,0 +1,144 @@
+# Module to set default compiler warnings.
+#
+# File adapted from Jason Turner's cpp_starter_project
+# https://github.com/lefticus/cpp_starter_project/blob/master/cmake/CompilerWarnings.cmake Using
+# INTERFACE targets is not so desirable as they need to be installed when building static libraries.
+
+function(mamba_target_add_compile_warnings target)
+    # Names of option parameters (without arguments)
+    set(options)
+    # Names of named parameters with a single argument
+    set(oneValueArgs WARNING_AS_ERROR)
+    # Names of named parameters with a multiple arguments
+    set(multiValueArgs)
+    cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
+
+    if(ARG_UNPARSED_ARGUMENTS)
+        message(
+            AUTHOR_WARNING
+            "Unrecoginzed options passed to ${CMAKE_CURRENT_FUNCTION}: "
+            "${ARG_UNPARSED_ARGUMENTS}"
+        )
+    endif()
+
+    set(
+        msvc_warnings
+        # External sever warnings
+        /experimental:external /external:W1
+        # Baseline reasonable warnings
+        /W4
+        # "identfier": conversion from "type1" to "type1", possible loss of data
+        /w14242
+        # "operator": conversion from "type1:field_bits" to "type2:field_bits", possible loss of
+        # data
+        /w14254
+        # "function": member function does not override any base class virtual member function
+        /w14263
+        # "classname": class has virtual functions, but destructor is not virtual instances of this
+        # class may not be destructed correctly
+        /w14265
+        # "operator": unsigned/negative constant mismatch
+        /w14287
+        # Nonstandard extension used: "variable": loop control variable declared in the for-loop is
+        # used outside the for-loop scope
+        /we4289
+        # "operator": expression is always "boolean_value"
+        /w14296
+        # "variable": pointer truncation from "type1" to "type2"
+        /w14311
+        # Expression before comma evaluates to a function which is missing an argument list
+        /w14545
+        # Function call before comma missing argument list
+        /w14546
+        # "operator": operator before comma has no effect; expected operator with side-effect
+        /w14547
+        # "operator": operator before comma has no effect; did you intend "operator"?
+        /w14549
+        # Expression has no effect; expected expression with side- effect
+        /w14555
+        # Pragma warning: there is no warning number "number"
+        /w14619
+        # Enable warning on thread un-safe static member initialization
+        /w14640
+        # Conversion from "type1" to "type_2" is sign-extended. This may cause unexpected runtime
+        # behavior.
+        /w14826
+        # Wide string literal cast to "LPSTR"
+        /w14905
+        # String literal cast to "LPWSTR"
+        /w14906
+        # Illegal copy-initialization; more than one user-defined conversion has been implicitly
+        # applied
+        /w14928
+    )
+
+    set(
+        clang_warnings
+        # Some default set of warnings
+        -Wall
+        # Reasonable and standard
+        -Wextra
+        # Warn the user if a variable declaration shadows one from a parent context
+        -Wshadow
+        # Warn the user if a class with virtual functions has a non-virtual destructor. This helps
+        # catch hard to track down memory errors
+        -Wnon-virtual-dtor
+        # Warn for c-style casts
+        -Wold-style-cast
+        # Warn for potential performance problem casts
+        -Wcast-align
+        # Warn on anything being unused
+        -Wunused
+        # Warn if you overload (not override) a virtual function
+        -Woverloaded-virtual
+        # Warn if non-standard C++ is used
+        -Wpedantic
+        # Warn on type conversions that may lose data
+        -Wconversion
+        # Warn on sign conversions
+        -Wsign-conversion
+        # Warn if a null dereference is detected
+        -Wnull-dereference
+        # Warn if float is implicit promoted to double
+        -Wdouble-promotion
+        # Warn on security issues around functions that format output (ie printf)
+        -Wformat=2
+        # Warn on code that cannot be executed
+        -Wunreachable-code
+        # Warn if a variable is used before being initialized
+        -Wuninitialized
+    )
+
+    if(${ARG_WARNING_AS_ERROR})
+        set(clang_warnings ${clang_warnings} -Werror)
+        set(msvc_warnings ${msvc_warnings} /WX)
+    endif()
+
+    set(
+        gcc_warnings
+        ${clang_warnings}
+        # Warn if identation implies blocks where blocks do not exist
+        -Wmisleading-indentation
+        # Warn if if / else chain has duplicated conditions
+        -Wduplicated-cond
+        # Warn if if / else branches have duplicated code
+        -Wduplicated-branches
+        # Warn about logical operations being used where bitwise were probably wanted
+        -Wlogical-op
+        # Warn if you perform a cast to the same type
+        -Wuseless-cast
+    )
+
+    if(MSVC)
+        set(warnings ${msvc_warnings})
+    elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")
+        set(warnings ${clang_warnings})
+    elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+        set(warnings ${clang_warnings})
+    else()
+        set(warnings ${gcc_warnings})
+    endif()
+
+    target_compile_options("${target}" PRIVATE ${warnings})
+
+endfunction()

libmamba/CMakeLists.txt

@@ -270,6 +270,11 @@ macro(libmamba_create_target target_name linkage deps_linkage output_name)
     # ======
     add_library(${target_name} ${linkage_upper} ${LIBMAMBA_SOURCES} ${LIBMAMBA_HEADERS})
 
+    mamba_target_add_compile_warnings(
+        ${target_name}
+        WARNING_AS_ERROR ${MAMBA_WARNING_AS_ERROR}
+    )
+
     if (${deps_linkage_upper} STREQUAL "STATIC")
         message("   -> Statically linking against libmamba (static) dependencies")
         if (UNIX)

libmamba/tests/CMakeLists.txt

@@ -44,6 +44,7 @@ set(LIBMAMBA_TEST_SRCS
 message(STATUS "Building libmamba C++ tests")
 
 add_executable(test_libmamba ${LIBMAMBA_TEST_SRCS})
+mamba_target_add_compile_warnings(test_libmamba WARNING_AS_ERROR ${MAMBA_WARNING_AS_ERROR})
 
 target_include_directories(
     test_libmamba

libmambapy/CMakeLists.txt

@@ -23,6 +23,8 @@ pybind11_add_module(bindings
     longpath.manifest
 )
 
+mamba_target_add_compile_warnings(bindings WARNING_AS_ERROR ${MAMBA_WARNING_AS_ERROR})
+
 target_link_libraries(bindings PUBLIC pybind11::pybind11 libmamba)
 set_property(TARGET bindings PROPERTY CXX_STANDARD 17)
 

mamba_package/CMakeLists.txt

@@ -47,6 +47,7 @@ endif ()
 # ================
 
 add_executable(mamba-package ${MAMBA_PACKAGE_SRCS} ${MAMBA_PACKAGE_HEADERS})
+mamba_target_add_compile_warnings(mamba-package WARNING_AS_ERROR ${MAMBA_WARNING_AS_ERROR})
 
 if (${MAMBA_PACKAGE_LINKAGE} STREQUAL "FULL_STATIC")
     target_link_libraries(mamba-package PRIVATE libmamba-full-static)

micromamba/CMakeLists.txt

@@ -53,6 +53,8 @@ set(MICROMAMBA_HEADERS
 
 add_executable(micromamba ${MICROMAMBA_SRCS} ${MICROMAMBA_HEADERS})
 
+mamba_target_add_compile_warnings(micromamba WARNING_AS_ERROR ${MAMBA_WARNING_AS_ERROR})
+
 if(NOT (TARGET libmamba OR TARGET libmamba-static OR TARGET libmamba-full-static))
     find_package(libmamba REQUIRED)
 endif ()