ci : add an option to fail on compile warning (#3952)

* feat(ci): add an option to fail on compile warning

* Update CMakeLists.txt

* minor : fix compile warnings

ggml-ci

* ggml : fix unreachable code warnings

ggml-ci

* ci : disable fatal warnings for windows, ios and tvos

* ggml : fix strncpy warning

* ci : disable fatal warnings for MPI build

* ci : add fatal warnings to ggml-ci

ggml-ci

---------

Co-authored-by: Georgi Gerganov <redacted>

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index ed292d6b8935d782ee036a97fb8a191e44856a47..03d76d45560cfeecdf0d8b1ff98fd659172b41c4 100644 (file)
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -37,6 +37,8 @@ jobs:
 
       - name: Build
         id: make_build
+        env:
+            LLAMA_FATAL_WARNINGS: 1
         run: |
           CC=gcc-8 make -j $(nproc)
 
@@ -65,7 +67,7 @@ jobs:
         run: |
           mkdir build
           cd build
-          cmake ..
+          cmake .. -DLLAMA_FATAL_WARNINGS=ON
           cmake --build . --config Release -j $(nproc)
 
       - name: Test
@@ -100,7 +102,7 @@ jobs:
         run: |
           mkdir build
           cd build
-          cmake .. -DLLAMA_SANITIZE_${{ matrix.sanitizer }}=ON -DCMAKE_BUILD_TYPE=${{ matrix.build_type }}
+          cmake .. -DLLAMA_FATAL_WARNINGS=ON -DLLAMA_SANITIZE_${{ matrix.sanitizer }}=ON -DCMAKE_BUILD_TYPE=${{ matrix.build_type }}
           cmake --build . --config ${{ matrix.build_type }} -j $(nproc)
 
       - name: Test
@@ -244,6 +246,8 @@ jobs:
 
       - name: Build
         id: make_build
+        env:
+            LLAMA_FATAL_WARNINGS: 1
         run: |
           LLAMA_NO_METAL=1 make -j $(sysctl -n hw.logicalcpu)
 
@@ -277,7 +281,7 @@ jobs:
           sysctl -a
           mkdir build
           cd build
-          cmake -DLLAMA_METAL=OFF ..
+          cmake -DLLAMA_FATAL_WARNINGS=ON -DLLAMA_METAL=OFF ..
           cmake --build . --config Release -j $(sysctl -n hw.logicalcpu)
 
       - name: Test

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2a922fdb316aaef07cda2ce66a907d5f7e77ca10..5ea4d4f1979dc5b8de2c9731050ae2aac55e377e 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -55,6 +55,9 @@ option(LLAMA_ALL_WARNINGS               "llama: enable all compiler warnings"
 option(LLAMA_ALL_WARNINGS_3RD_PARTY     "llama: enable all compiler warnings in 3rd party libs" OFF)
 option(LLAMA_GPROF                      "llama: enable gprof"                                   OFF)
 
+# build
+option(LLAMA_FATAL_WARNINGS             "llama: enable -Werror flag"                            OFF)
+
 # sanitizers
 option(LLAMA_SANITIZE_THREAD            "llama: enable thread sanitizer"                        OFF)
 option(LLAMA_SANITIZE_ADDRESS           "llama: enable address sanitizer"                       OFF)
@@ -142,6 +145,14 @@ set(THREADS_PREFER_PTHREAD_FLAG ON)
 find_package(Threads REQUIRED)
 include(CheckCXXCompilerFlag)
 
+if (LLAMA_FATAL_WARNINGS)
+    if (CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+        add_compile_options(-Werror)
+    elseif (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
+        add_compile_options(/WX)
+    endif()
+endif()
+
 # enable libstdc++ assertions for debug builds
 if (CMAKE_SYSTEM_NAME MATCHES "Linux")
     add_compile_definitions($<$<CONFIG:Debug>:_GLIBCXX_ASSERTIONS>)

diff --git a/Makefile b/Makefile
index 0a2070b539df83f3e130e934377f9c38e81655e3..90179860623d01a0f5912b0acc79ac6d2639ed91 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -215,6 +215,35 @@ MK_CFLAGS    += $(WARN_FLAGS) -Wshadow -Wstrict-prototypes -Wpointer-arith -Wmis
                                -Werror=implicit-function-declaration
 MK_CXXFLAGS  += $(WARN_FLAGS) -Wmissing-declarations -Wmissing-noreturn
 
+ifeq ($(LLAMA_FATAL_WARNINGS),1)
+       MK_CFLAGS += -Werror
+       MK_CXXFLAGS += -Werror
+endif
+
+ifeq ($(CC_IS_CLANG), 1)
+       # clang options
+       MK_CFLAGS        += -Wunreachable-code-break -Wunreachable-code-return
+       MK_HOST_CXXFLAGS += -Wunreachable-code-break -Wunreachable-code-return -Wmissing-prototypes -Wextra-semi
+
+       ifneq '' '$(and $(CC_IS_LLVM_CLANG),$(filter 1,$(shell expr $(CC_VER) \>= 030800)))'
+               MK_CFLAGS += -Wdouble-promotion
+       endif
+       ifneq '' '$(and $(CC_IS_APPLE_CLANG),$(filter 1,$(shell expr $(CC_VER) \>= 070300)))'
+               MK_CFLAGS += -Wdouble-promotion
+       endif
+else
+       # gcc options
+       MK_CFLAGS        += -Wdouble-promotion
+       MK_HOST_CXXFLAGS += -Wno-array-bounds
+
+       ifeq ($(shell expr $(CC_VER) \>= 070100), 1)
+               MK_HOST_CXXFLAGS += -Wno-format-truncation
+       endif
+       ifeq ($(shell expr $(CC_VER) \>= 080100), 1)
+               MK_HOST_CXXFLAGS += -Wextra-semi
+       endif
+endif
+
 # this version of Apple ld64 is buggy
 ifneq '' '$(findstring dyld-1015.7,$(shell $(CC) $(LDFLAGS) -Wl,-v 2>&1))'
        MK_CPPFLAGS += -DHAVE_BUGGY_APPLE_LINKER

diff --git a/ci/run.sh b/ci/run.sh
index 979b4a79391374a4da685722d1bae8da59054599..b94658c96c5a4942eaee28be6ddfdf90b5af6781 100755 (executable)
--- a/ci/run.sh
+++ b/ci/run.sh
@@ -33,7 +33,7 @@ sd=`dirname $0`
 cd $sd/../
 SRC=`pwd`
 
-CMAKE_EXTRA=""
+CMAKE_EXTRA="-DLLAMA_FATAL_WARNINGS=ON"
 
 if [ ! -z ${GG_BUILD_METAL} ]; then
     CMAKE_EXTRA="${CMAKE_EXTRA} -DLLAMA_METAL_SHADER_DEBUG=ON"

diff --git a/examples/export-lora/export-lora.cpp b/examples/export-lora/export-lora.cpp
index 2f7be8a1344faf3a2069f30c5d0c25bdb37756fa..08413f57e4c3a3ec81935a4ce382811638771ad4 100644 (file)
--- a/examples/export-lora/export-lora.cpp
+++ b/examples/export-lora/export-lora.cpp
@@ -7,8 +7,6 @@
 #include <string>
 #include <thread>
 
-static const size_t tensor_alignment = 32;
-
 struct lora_info {
     std::string filename;
     float scale;

diff --git a/ggml-backend.c b/ggml-backend.c
index 87eea84402e926b8b7ae2a2230b021a0a208a7b2..d019d813ad5f070f5745515fbf9d8c9172172cd4 100644 (file)
--- a/ggml-backend.c
+++ b/ggml-backend.c
@@ -1006,6 +1006,7 @@ static int ggml_backend_sched_backend_from_buffer(ggml_backend_sched_t sched, gg
         }
     }
     GGML_ASSERT(false && "tensor buffer type not supported by any backend");
+    return -1; // silence warning
 }
 
 #if 0

diff --git a/ggml-metal.m b/ggml-metal.m
index c1d8e2de81e023aa277b059ce94cac506f6b2a8b..6e76f8bedb50b6e40841b727a44c3a6870694865 100644 (file)
--- a/ggml-metal.m
+++ b/ggml-metal.m
@@ -176,7 +176,7 @@ struct ggml_metal_context {
 // MSL code
 // TODO: move the contents here when ready
 //       for now it is easier to work in a separate file
-//static NSString * const msl_library_source = @"see metal.metal";
+// static NSString * const msl_library_source = @"see metal.metal";
 
 // Here to assist with NSBundle Path Hack
 @interface GGMLMetalClass : NSObject

diff --git a/ggml.c b/ggml.c
index 4e302fb7de2f48c2e4d5db594ec492eb3e671f21..264cfd705cd378aac42cf65b09e451a9c80fad79 100644 (file)
--- a/ggml.c
+++ b/ggml.c
@@ -868,7 +868,7 @@ do {                                                              \
     const __m128 t0 = _mm_add_ps(_mm256_castps256_ps128(x[0]),    \
                                  _mm256_extractf128_ps(x[0], 1)); \
     const __m128 t1 = _mm_hadd_ps(t0, t0);                        \
-    res = _mm_cvtss_f32(_mm_hadd_ps(t1, t1));                     \
+    res = (ggml_float) _mm_cvtss_f32(_mm_hadd_ps(t1, t1));        \
 } while (0)
 // TODO: is this optimal ?
 
@@ -1149,7 +1149,7 @@ inline static void __wasm_f16x4_store(ggml_fp16_t * p, v128_t x) {
         x[i] = _mm_add_ps(x[i], x[offset+i]);                     \
     }                                                             \
     const __m128 t0 = _mm_hadd_ps(x[0], x[0]);                    \
-    res = _mm_cvtss_f32(_mm_hadd_ps(t0, t0));                     \
+    res = (ggml_float) _mm_cvtss_f32(_mm_hadd_ps(t0, t0));        \
 }
 // TODO: is this optimal ?
 
@@ -2086,6 +2086,7 @@ void ggml_numa_init(enum ggml_numa_strategy numa_flag) {
         }
     }
 #else
+    GGML_UNUSED(numa_flag);
     // TODO
 #endif
 }
@@ -3219,7 +3220,7 @@ const char * ggml_get_name(const struct ggml_tensor * tensor) {
 }
 
 struct ggml_tensor * ggml_set_name(struct ggml_tensor * tensor, const char * name) {
-    strncpy(tensor->name, name, sizeof(tensor->name));
+    strncpy(tensor->name, name, sizeof(tensor->name) - 1);
     tensor->name[sizeof(tensor->name) - 1] = '\0';
     return tensor;
 }
@@ -18575,7 +18576,9 @@ static enum ggml_opt_result linesearch_backtracking(
         (*step) *= width;
     }
 
-    GGML_UNREACHABLE();
+    GGML_ASSERT(false && "line search failed");
+
+    return GGML_LINESEARCH_FAIL;
 }
 
 static enum ggml_opt_result ggml_opt_lbfgs(
@@ -18843,7 +18846,9 @@ static enum ggml_opt_result ggml_opt_lbfgs(
         step[0] = 1.0;
     }
 
-    GGML_UNREACHABLE();
+    GGML_ASSERT(false && "lbfgs failed");
+
+    return GGML_OPT_DID_NOT_CONVERGE;
 }
 
 struct ggml_opt_params ggml_opt_default_params(enum ggml_opt_type type) {