build: enable Werror on non-Windows

Because of our changes recently, we can now build the project with -Werror. However, this doesn't enable all warnings. We should add warnings to the list of flagged warnings so as to forbid more and more bad code. Windows is simply too noisy to enable -Werror for right now.
kuzudb · Nov 2, 2023 · 8f0f871 · 8f0f871
1 parent 7b408f1
commit 8f0f871
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 20 deletions.
diff --git a/.github/workflows/ci-workflow.yml b/.github/workflows/ci-workflow.yml
@@ -1,5 +1,8 @@
 name: CI
 
+env:
+  WERROR: 1
+
 on:
   pull_request:
     branches:
@@ -180,6 +183,7 @@ jobs:
       CARGO_BUILD_JOBS: 18
       NUM_THREADS: 18
       TEST_JOBS: 9
+      WERROR: 0
     steps:
       - uses: actions/checkout@v3
 

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -6,13 +6,18 @@ find_package(Threads REQUIRED)
 
 set(CMAKE_FIND_PACKAGE_RESOLVE_SYMLINKS TRUE)
 set(CMAKE_CXX_STANDARD 20)
-set(CMAKE_CXX_STANDARD_REQUIRED True)
+set(CMAKE_CXX_STANDARD_REQUIRED TRUE)
 set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 if(NOT APPLE)
     set(CMAKE_CXX_VISIBILITY_PRESET hidden)
     set(CMAKE_VISIBILITY_INLINES_HIDDEN ON)
 endif()
 
+option(ENABLE_WERROR "Treat all warnings as errors" FALSE)
+if(ENABLE_WERROR)
+    set(CMAKE_COMPILE_WARNING_AS_ERROR TRUE)
+endif()
+
 # Detect OS and architecture, copied from DuckDB
 set(OS_NAME "unknown")
 set(OS_ARCH "amd64")

diff --git a/Makefile b/Makefile
@@ -5,6 +5,7 @@ FORCE_COLOR=
 NUM_THREADS ?= 1
 TEST_JOBS ?= 10
 SANITIZER_FLAG=
+WERROR_FLAG=
 ROOT_DIR=$(CURDIR)
 
 export CMAKE_BUILD_PARALLEL_LEVEL=$(NUM_THREADS)
@@ -30,6 +31,10 @@ ifeq ($(UBSAN), 1)
 	SANITIZER_FLAG=-DENABLE_ADDRESS_SANITIZER=FALSE -DENABLE_THREAD_SANITIZER=TRUE -DENABLE_UBSAN=TRUE
 endif
 
+ifeq ($(WERROR), 1)
+	WERROR_FLAG=-DENABLE_WERROR=TRUE
+endif
+
 ifeq ($(OS),Windows_NT)
 define mkdirp
 	(if not exist "$(1)" mkdir "$(1)")
@@ -42,61 +47,61 @@ endif
 
 release:
 	$(call mkdirp,build/release) && cd build/release && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_BUILD_TYPE=Release ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release ../.. && \
 	cmake --build . --config Release
 
 debug:
 	$(call mkdirp,build/debug) && cd build/debug && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_BUILD_TYPE=Debug ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Debug ../.. && \
 	cmake --build . --config Debug
 
 all:
 	$(call mkdirp,build/release) && cd build/release && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=TRUE -DBUILD_BENCHMARK=TRUE -DBUILD_NODEJS=TRUE -DBUILD_EXAMPLES=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=TRUE -DBUILD_BENCHMARK=TRUE -DBUILD_NODEJS=TRUE -DBUILD_EXAMPLES=TRUE ../.. && \
 	cmake --build . --config Release
 
 example:
 	$(call mkdirp,build/release) && cd build/release && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_EXAMPLES=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_EXAMPLES=TRUE ../.. && \
 	cmake --build . --config Release
 
 alldebug:
 	$(call mkdirp,build/debug) && cd build/debug && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_BUILD_TYPE=Debug -DBUILD_TESTS=TRUE -DBUILD_BENCHMARK=TRUE -DBUILD_NODEJS=TRUE -DBUILD_EXAMPLES=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Debug -DBUILD_TESTS=TRUE -DBUILD_BENCHMARK=TRUE -DBUILD_NODEJS=TRUE -DBUILD_EXAMPLES=TRUE ../.. && \
 	cmake --build . --config Debug
 
 benchmark:
 	$(call mkdirp,build/release) && cd build/release && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_BENCHMARK=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_BENCHMARK=TRUE ../.. && \
 	cmake --build . --config Release
 
 nodejs:
 	$(call mkdirp,build/release) && cd build/release && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_NODEJS=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_NODEJS=TRUE ../.. && \
 	cmake --build . --config Release
 
 java:
 	$(call mkdirp,build/release) && cd build/release && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_JAVA=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_JAVA=TRUE ../.. && \
 	cmake --build . --config Release
 
 test:
 	$(call mkdirp,build/release) && cd build/release && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=TRUE ../.. && \
 	cmake --build . --config Release
 	cd $(ROOT_DIR)/build/release/test && \
 	ctest --output-on-failure -j ${TEST_JOBS}
 
 lcov:
 	$(call mkdirp,build/release) && cd build/release && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=TRUE -DBUILD_NODEJS=TRUE -DBUILD_LCOV=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=TRUE -DBUILD_NODEJS=TRUE -DBUILD_LCOV=TRUE ../.. && \
 	cmake --build . --config Release
 	cd $(ROOT_DIR)/build/release/test && \
 	ctest --output-on-failure -j ${TEST_JOBS}
 
 clangd:
 	$(call mkdirp,build/release) && cd build/release && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) -DCMAKE_EXPORT_COMPILE_COMMANDS=1 ../..
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_EXPORT_COMPILE_COMMANDS=1 ../..
 
 tidy: clangd
 	run-clang-tidy -p build/release -quiet -j $(NUM_THREADS) \

diff --git a/src/common/types/int128_t.cpp b/src/common/types/int128_t.cpp
@@ -427,8 +427,8 @@ bool CastInt128ToFloating(int128_t input, REAL_T& result) {
         result = -REAL_T(function::NumericLimits<uint64_t>::maximum() - input.low) - 1;
         break;
     default:
-        result =
-            REAL_T(input.high) * function::NumericLimits<uint64_t>::maximum() + REAL_T(input.low);
+        result = REAL_T(input.high) * REAL_T(function::NumericLimits<uint64_t>::maximum()) +
+                 REAL_T(input.low);
         break;
     }
     return true;

diff --git a/tools/shell/embedded_shell.cpp b/tools/shell/embedded_shell.cpp
@@ -6,11 +6,7 @@
 #include <regex>
 
 #include "catalog/catalog.h"
-#include "common/logging_level_utils.h"
 #include "common/string_utils.h"
-#include "common/type_utils.h"
-#include "json.hpp"
-#include "processor/result/factorized_table.h"
 #include "utf8proc.h"
 #include "utf8proc_wrapper.h"
 
@@ -260,7 +256,7 @@ void EmbeddedShell::run() {
     }
 }
 
-void EmbeddedShell::interruptHandler(int  /*signal*/) {
+void EmbeddedShell::interruptHandler(int /*signal*/) {
     globalConnection->interrupt();
 }
 
@@ -352,7 +348,7 @@ void EmbeddedShell::printExecutionResult(QueryResult& queryResult) const {
         if (numTuples == 1) {
             printf("(1 tuple)\n");
         } else {
-            printf("(%lu tuples)\n", numTuples);
+            printf("(%" PRIu64 " tuples)\n", numTuples);
         }
         printf("Time: %.2fms (compiling), %.2fms (executing)\n", querySummary->getCompilingTime(),
             querySummary->getExecutionTime());