build: add flag to treat warnings as errors and enable in CI (#61)

* build: add flag to treat warnings as errors and enable in CI

This PR adds an option to our CMake files that allow treating compiler
warnings as errors in the targets from this repository and enables that
option in CI. This will enforce that we fix compiler warnings before
merging PRs, which should help to keep our code clean. Also fix a final
warning.

Signed-off-by: Ingo Müller <[email protected]>

Author: ingomueller-net

.github/workflows/build_and_test.yml

@@ -85,6 +85,7 @@ jobs:
           -DMLIR_INCLUDE_INTEGRATION_TESTS=ON \
           -DMLIR_ENABLE_BINDINGS_PYTHON=ON \
           -DMLIR_ENABLE_PYTHON_BENCHMARKS=ON \
+          -DSUBSTRAIT_MLIR_COMPILE_WARNING_AS_ERROR=ON \
           -S${SUBSTRAIT_MLIR_MAIN_SRC_DIR}/third_party/llvm-project/llvm \
           -B${SUBSTRAIT_MLIR_MAIN_BINARY_DIR} -G Ninja
         echo "PYTHONPATH=${PYTHONPATH}:${SUBSTRAIT_MLIR_MAIN_BINARY_DIR}/tools/substrait_mlir/python_packages" | tee -a $GITHUB_ENV

CMakeLists.txt

@@ -5,6 +5,14 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
     "-DLLVM_EXTERNAL_SUBSTRAIT_MLIR_SOURCE_DIR=${CMAKE_CURRENT_SOURCE_DIR}")
 endif()
 
+################################################################################
+# CMake options specific to this sub-project
+################################################################################
+
+option(SUBSTRAIT_MLIR_COMPILE_WARNING_AS_ERROR
+       "value for 'CMAKE_COMPILE_WARNING_AS_ERROR' for Substrait MLIR targets"
+       OFF)
+
 ################################################################################
 # Set up dependencies
 ################################################################################
@@ -69,6 +77,9 @@ add_subdirectory(third_party)
 ################################################################################
 add_custom_target(substrait-mlir-all)
 
+# Set default value for COMPILE_WARNING_AS_ERROR for our targets.
+set(CMAKE_COMPILE_WARNING_AS_ERROR ${SUBSTRAIT_MLIR_COMPILE_WARNING_AS_ERROR})
+
 add_subdirectory(lib)
 add_subdirectory(include)
 add_subdirectory(python)

README.md

@@ -285,6 +285,7 @@ cmake \
   -DMLIR_INCLUDE_INTEGRATION_TESTS=ON \
   -DMLIR_ENABLE_BINDINGS_PYTHON=ON \
   -DMLIR_ENABLE_PYTHON_BENCHMARKS=ON \
+  -DSUBSTRAIT_MLIR_COMPILE_WARNING_AS_ERROR=ON \
   -S${SUBSTRAIT_MLIR_SOURCE_DIR}/third_party/llvm-project/llvm \
   -B${SUBSTRAIT_MLIR_BUILD_DIR} \
   -G Ninja

lib/Target/SubstraitPB/Export.cpp

@@ -689,7 +689,7 @@ SubstraitExporter::exportOperation(LiteralOp op) {
   }
   // `TimeType`.
   else if (auto timeType = dyn_cast<TimeType>(literalType)) {
-    literal->set_time(value.cast<TimeAttr>().getValue());
+    literal->set_time(mlir::cast<TimeAttr>(value).getValue());
   } else
     op->emitOpError("has unsupported value");