maint: Enable `-Werror` compiler flag for GCC, Clang and AppleClang (#3611)

CMakeLists.txt

@@ -19,7 +19,11 @@ option(BUILD_LIBMAMBA_TESTS "Build libmamba C++ tests" OFF)
 option(BUILD_MAMBA "Build mamba" 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)
+if(MSVC)
+    option(MAMBA_WARNING_AS_ERROR "Treat compiler warnings as errors" OFF)
+else()
+    option(MAMBA_WARNING_AS_ERROR "Treat compiler warnings as errors" ON)
+endif()
 set(
     MAMBA_LTO
     "Default"

cmake/CompilerWarnings.cmake

@@ -109,6 +109,21 @@ function(mamba_target_add_compile_warnings target)
         -Wuninitialized
     )
 
+    # It seems that these flags are leaked to CXXFLAGS: `-framework CoreFoundation` `-framework
+    # CoreServices` `-framework Security` `-framework Kerberos` `-fno-merge-constants`
+    #
+    # These flags give compiler warnings/errors: `unused-command-line-argument` and
+    # `ignored-optimization-argument` So we disable these warnings/errors for all the files
+    #
+    # This might be happening during `conda-build` and might be a bug in
+    # https://github.com/conda-forge/clang-compiler-activation-feedstock
+    if(APPLE AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+        set(
+            clang_warnings
+            ${clang_warnings} -Wno-unused-command-line-argument -Wno-ignored-optimization-argument
+        )
+    endif()
+
     if(${ARG_WARNING_AS_ERROR})
         set(clang_warnings ${clang_warnings} -Werror)
         set(msvc_warnings ${msvc_warnings} /WX)

libmamba/CMakeLists.txt

@@ -264,6 +264,14 @@ set(
     ${LIBMAMBA_SOURCE_DIR}/api/shell.cpp
     ${LIBMAMBA_SOURCE_DIR}/api/update.cpp
 )
+# TODO: remove when switch to C++20
+if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")
+    # This file uses capturing structured bindings, which was fixed in C++20
+    set_source_files_properties(
+        ${LIBMAMBA_SOURCE_DIR}/download/mirror_impl.cpp
+        PROPERTIES COMPILE_FLAGS -Wno-c++20-extensions
+    )
+endif()
 
 foreach(script ${SHELL_SCRIPTS})
     list(APPEND LIBMAMBA_SOURCES shell_scripts/${script}.cpp)

libmamba/ext/solv-cpp/src/pool.cpp

@@ -93,10 +93,21 @@ namespace solv
 
     namespace
     {
+// This function is only used in `assert()` expressions
+// That's why it might get reported as unused in Release builds
+#if defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-function"
+#endif
+
         auto is_reldep(::Id id) -> bool
         {
             return ISRELDEP(static_cast<std::make_unsigned_t<::Id>>(id)) != 0;
         }
+
+#if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
     }
 
     auto ObjPoolView::get_string(StringId id) const -> std::string_view

libmamba/include/mamba/util/conditional.hpp

@@ -31,9 +31,7 @@ namespace mamba::util
         }
         else
         {
-            using int_t = Int;
-            return static_cast<int_t>(condition) * true_val
-                   + (int_t(1) - static_cast<int_t>(condition)) * false_val;
+            return condition ? true_val : false_val;
         }
     }
 }

libmamba/src/core/run.cpp

@@ -186,6 +186,13 @@ namespace mamba
         return !other_processes_with_same_name.empty();
     }
 
+// This ctor only uses proc_dir_lock in `assert()` expression
+// That's why it might get reported as unused in Release builds
+#if defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-parameter"
+#endif
+
     ScopedProcFile::ScopedProcFile(
         const Context& context,
         const std::string& name,
@@ -217,6 +224,10 @@ namespace mamba
         pid_file << file_json;
     }
 
+#if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
+
     ScopedProcFile::~ScopedProcFile()
     {
         const auto lock = lock_proc_dir();

libmamba/src/core/thread_utils.cpp

@@ -29,6 +29,29 @@ namespace mamba
 #ifndef _WIN32
     namespace
     {
+#if defined(__APPLE__) && defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wsign-conversion"
+#endif
+
+        // `sigaddset` might be implemented as a macro calling `__sigbits(int)` function
+        // At the same time `sigset_t` might be `unsigned int`
+        // This causes compiler warning
+        sigset_t get_sigset()
+        {
+            // block signals in this thread and subsequently
+            // spawned threads
+            sigset_t sigset;
+            sigemptyset(&sigset);
+            sigaddset(&sigset, SIGINT);
+            // sigaddset(&sigset, SIGTERM);
+            return sigset;
+        }
+
+#if defined(__APPLE__) && defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
+
         std::thread::native_handle_type sig_recv_thread;
         std::atomic<bool> receiver_exists(false);
     }
@@ -74,12 +97,7 @@ namespace mamba
     {
         stop_receiver_thread();
 
-        // block signals in this thread and subsequently
-        // spawned threads
-        sigset_t sigset;
-        sigemptyset(&sigset);
-        sigaddset(&sigset, SIGINT);
-        // sigaddset(&sigset, SIGTERM);
+        sigset_t sigset = get_sigset();
         pthread_sigmask(SIG_BLOCK, &sigset, nullptr);
         std::thread receiver(handler, sigset);
         sig_recv_thread = receiver.native_handle();

libmamba/src/core/util.cpp

@@ -971,6 +971,13 @@ namespace mamba
                       << "'";
         }
 
+// This function is only used in `assert()` expressions
+// That's why it might get reported as unused in Release builds
+#if defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-function"
+#endif
+
         bool is_lockfile_locked(const LockFileOwner& lockfile)
         {
 #ifdef _WIN32
@@ -981,6 +988,10 @@ namespace mamba
 #endif
         }
 
+#if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
+
         class LockedFilesRegistry
         {
         public:

libmamba/src/solver/libsolv/matcher.cpp

@@ -193,7 +193,7 @@ namespace mamba::solver::libsolv
             return { std::cref(it->second) };
         }
 
-        return specs::Channel::resolve(std::move(uc), channel_params())
+        return specs::Channel::resolve(uc, channel_params())
             .transform(
                 [&](channel_list&& chan)
                 {

libmamba/src/solver/problems_graph.cpp

@@ -264,6 +264,12 @@ namespace mamba::solver
             return CompressedProblemsGraph::NamedList<O>(tmp.begin(), tmp.end());
         }
 
+// GCC reports dangling reference when using std::invoke with data members
+#if defined(__GNUC__) && !defined(__clang__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdangling-reference"
+#endif
+
         template <typename T>
         auto invoke_version(T&& e) -> decltype(auto)
         {
@@ -328,6 +334,10 @@ namespace mamba::solver
             }
         }
 
+#if defined(__GNUC__) && !defined(__clang__)
+#pragma GCC diagnostic pop
+#endif
+
         /**
          * Detect if a type has a ``name`` member (function).
          */

libmambapy/CMakeLists.txt

@@ -32,6 +32,14 @@ pybind11_add_module(
     src/libmambapy/bindings/solver.cpp
     src/libmambapy/bindings/solver_libsolv.cpp
 )
+# TODO: remove when `SubdirData::cache_path()` is removed
+if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")
+    # This file uses capturing structured bindings, which was fixed in C++20
+    set_source_files_properties(
+        src/libmambapy/bindings/legacy.cpp
+        PROPERTIES COMPILE_FLAGS -Wno-error=deprecated-declarations
+    )
+endif()
 
 target_include_directories(bindings PRIVATE src/libmambapy/bindings)