replace std::assert with KU_ASSERT

This also enables a parameter RUNTIME_CHECKS that can be passed to the Makefile to force these checks to be on. This also enables a CI check to make sure we consistently use KU_ASSERT and not std::assert.

Closes #2283.

.github/workflows/ci-workflow.yml

@@ -1,6 +1,7 @@
 name: CI
 
 env:
+  RUNTIME_CHECKS: 1
   WERROR: 1
 
 on:
@@ -23,7 +24,7 @@ concurrency:
 jobs:
   gcc-build-test:
     name: gcc build & test
-    needs: [clang-formatting-check, header-include-guard-check]
+    needs: [clang-formatting-check, include-guard-and-no-std-assert]
     runs-on: kuzu-self-hosted-testing
     env:
       NUM_THREADS: 32
@@ -141,7 +142,7 @@ jobs:
 
   clang-build-test:
     name: clang build, test & tidy
-    needs: [clang-formatting-check, header-include-guard-check]
+    needs: [clang-formatting-check, include-guard-and-no-std-assert]
     runs-on: kuzu-self-hosted-testing
     env:
       NUM_THREADS: 32
@@ -180,7 +181,7 @@ jobs:
 
   msvc-build-test:
     name: msvc build & test
-    needs: [clang-formatting-check, header-include-guard-check]
+    needs: [clang-formatting-check, include-guard-and-no-std-assert]
     runs-on: self-hosted-windows
     env:
       # Shorten build path as much as possible
@@ -223,14 +224,17 @@ jobs:
           call "C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Auxiliary\Build\vcvars64.bat"
           make example
 
-  header-include-guard-check:
-    name: header include guard check
+  include-guard-and-no-std-assert:
+    name: include guard & no std::assert check
     runs-on: ubuntu-22.04
     steps:
       - uses: actions/checkout@v3
 
       - name: Check headers for include guards
-        run: ./scripts/check-headers.sh src/include
+        run: ./scripts/check-include-guards.sh src/include
+
+      - name: Checks files for std::assert
+        run: ./scripts/check-no-std-assert.sh src
 
   clang-formatting-check:
     name: clang-format check
@@ -280,7 +284,7 @@ jobs:
 
   macos-build-test:
     name: apple clang build & test
-    needs: [clang-formatting-check, header-include-guard-check, rustfmt-check]
+    needs: [clang-formatting-check, include-guard-and-no-std-assert, rustfmt-check]
     runs-on: self-hosted-mac-x64
     env:
       NUM_THREADS: 32

CMakeLists.txt

@@ -76,6 +76,7 @@ set(INSTALL_CMAKE_DIR
 option(ENABLE_ADDRESS_SANITIZER "Enable address sanitizer." FALSE)
 option(ENABLE_THREAD_SANITIZER "Enable thread sanitizer." FALSE)
 option(ENABLE_UBSAN "Enable undefined behavior sanitizer." FALSE)
+option(ENABLE_RUNTIME_CHECKS "Enable runtime coherency checks (e.g. asserts)" FALSE)
 option(USE_SYSTEM_ARROW "Use system version of arrow" FALSE)
 if(MSVC)
     # Required for M_PI on Windows
@@ -129,6 +130,11 @@ if(${ENABLE_UBSAN})
         set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-common -fpermissive")
     endif()
 endif()
+
+if(${ENABLE_RUNTIME_CHECKS})
+    add_compile_definitions(KUZU_RUNTIME_CHECKS)
+endif()
+
 option(BUILD_PYTHON_API "Build Python API." TRUE)
 option(BUILD_SHELL "Build Interactive Shell" TRUE)
 option(BUILD_TESTS "Build C++ and Python tests." FALSE)

Makefile

@@ -1,13 +1,14 @@
 .PHONY: release debug test benchmark all alldebug clean clean-all
 
-GENERATOR=
+CLANGD_DIAGNOSTIC_INSTANCES ?= 4
 FORCE_COLOR=
+GENERATOR=
 NUM_THREADS ?= 1
-TEST_JOBS ?= 10
+ROOT_DIR=$(CURDIR)
+RUNTIME_CHECK_FLAG=
 SANITIZER_FLAG=
+TEST_JOBS ?= 10
 WERROR_FLAG=
-CLANGD_DIAGNOSTIC_INSTANCES ?= 4
-ROOT_DIR=$(CURDIR)
 
 export CMAKE_BUILD_PARALLEL_LEVEL=$(NUM_THREADS)
 
@@ -32,6 +33,9 @@ ifeq ($(UBSAN), 1)
 	SANITIZER_FLAG=-DENABLE_ADDRESS_SANITIZER=FALSE -DENABLE_THREAD_SANITIZER=TRUE -DENABLE_UBSAN=TRUE
 endif
 
+ifeq ($(RUNTIME_CHECKS), 1)
+	RUNTIME_CHECK_FLAG=-DENABLE_RUNTIME_CHECKS=TRUE
+endif
 ifeq ($(WERROR), 1)
 	WERROR_FLAG=-DENABLE_WERROR=TRUE
 endif
@@ -48,61 +52,61 @@ endif
 
 release:
 	$(call mkdirp,build/release) && cd build/release && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) $(RUNTIME_CHECK_FLAG) -DCMAKE_BUILD_TYPE=Release ../.. && \
 	cmake --build . --config Release
 
 debug:
 	$(call mkdirp,build/debug) && cd build/debug && \
-	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Debug ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) $(RUNTIME_CHECK_FLAG) -DCMAKE_BUILD_TYPE=Debug ../.. && \
 	cmake --build . --config Debug
 
 all:
 	$(call mkdirp,build/release) && cd build/release && \
-	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 $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) $(RUNTIME_CHECK_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) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_EXAMPLES=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) $(RUNTIME_CHECK_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) $(WERROR_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) $(RUNTIME_CHECK_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) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_BENCHMARK=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) $(RUNTIME_CHECK_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) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_NODEJS=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) $(RUNTIME_CHECK_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) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_JAVA=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) $(RUNTIME_CHECK_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) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) $(RUNTIME_CHECK_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) $(WERROR_FLAG) -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=TRUE -DBUILD_NODEJS=TRUE -DBUILD_LCOV=TRUE ../.. && \
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) $(RUNTIME_CHECK_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) $(WERROR_FLAG) -DCMAKE_EXPORT_COMPILE_COMMANDS=1 ../..
+	cmake $(GENERATOR) $(FORCE_COLOR) $(SANITIZER_FLAG) $(WERROR_FLAG) $(RUNTIME_CHECK_FLAG) -DCMAKE_EXPORT_COMPILE_COMMANDS=1 ../..
 
 tidy: clangd
 	run-clang-tidy -p build/release -quiet -j $(NUM_THREADS) \

scripts/check-no-std-assert.sh

@@ -0,0 +1,2 @@
+#!/bin/sh
+! grep --color=auto -r "include <assert.h>\|include <cassert>" $1

src/binder/bind/bind_copy.cpp

@@ -134,7 +134,7 @@ std::unique_ptr<BoundStatement> Binder::bindCopyRelFrom(
     std::unique_ptr<ReaderConfig> readerConfig, TableSchema* tableSchema) {
     // For table with SERIAL columns, we need to read in serial from files.
     auto containsSerial = bindContainsSerial(tableSchema);
-    assert(containsSerial == false);
+    KU_ASSERT(containsSerial == false);
     auto columnsToRead = bindExpectedRelFileColumns(tableSchema, *readerConfig);
     auto relID = createVariable(std::string(Property::INTERNAL_ID_NAME), LogicalTypeID::INT64);
     auto boundFileScanInfo = std::make_unique<BoundFileScanInfo>(
@@ -218,10 +218,10 @@ expression_vector Binder::bindExpectedRelFileColumns(
         readerConfig.columnNames.push_back(dstColumnName);
         auto srcTable =
             catalog.getReadOnlyVersion()->getTableSchema(relTableSchema->getSrcTableID());
-        assert(srcTable->tableType == TableType::NODE);
+        KU_ASSERT(srcTable->tableType == TableType::NODE);
         auto dstTable =
             catalog.getReadOnlyVersion()->getTableSchema(relTableSchema->getDstTableID());
-        assert(dstTable->tableType == TableType::NODE);
+        KU_ASSERT(dstTable->tableType == TableType::NODE);
         auto srcPKColumnType =
             reinterpret_cast<NodeTableSchema*>(srcTable)->getPrimaryKey()->getDataType()->copy();
         if (srcPKColumnType->getLogicalTypeID() == LogicalTypeID::SERIAL) {

src/binder/bind/bind_file_scan.cpp

@@ -112,7 +112,7 @@ std::unique_ptr<CSVReaderConfig> Binder::bindParsingOptions(
 
         auto copyOptionExpression = parsingOption.second.get();
         auto boundCopyOptionExpression = expressionBinder.bindExpression(*copyOptionExpression);
-        assert(boundCopyOptionExpression->expressionType == LITERAL);
+        KU_ASSERT(boundCopyOptionExpression->expressionType == LITERAL);
         if (isValidBoolParsingOption) {
             if (boundCopyOptionExpression->dataType.getLogicalTypeID() != LogicalTypeID::BOOL) {
                 throw BinderException(

src/binder/bind/bind_query.cpp

@@ -16,7 +16,7 @@ std::unique_ptr<BoundRegularQuery> Binder::bindQuery(const RegularQuery& regular
         normalizedSingleQueries.push_back(bindSingleQuery(*regularQuery.getSingleQuery(i)));
     }
     validateUnionColumnsOfTheSameType(normalizedSingleQueries);
-    assert(!normalizedSingleQueries.empty());
+    KU_ASSERT(!normalizedSingleQueries.empty());
     auto boundRegularQuery = std::make_unique<BoundRegularQuery>(
         regularQuery.getIsUnionAll(), normalizedSingleQueries[0]->getStatementResult()->copy());
     for (auto& normalizedSingleQuery : normalizedSingleQueries) {

src/binder/bind/bind_reading_clause.cpp

@@ -199,7 +199,7 @@ expression_vector Binder::createColumnExpressions(common::ReaderConfig& readerCo
 void Binder::validateColumnTypes(const std::vector<std::string>& columnNames,
     const std::vector<std::unique_ptr<LogicalType>>& expectedColumnTypes,
     const std::vector<std::unique_ptr<LogicalType>>& detectedColumnTypes) {
-    assert(expectedColumnTypes.size() == detectedColumnTypes.size());
+    KU_ASSERT(expectedColumnTypes.size() == detectedColumnTypes.size());
     for (auto i = 0; i < expectedColumnTypes.size(); ++i) {
         if (*expectedColumnTypes[i] != *detectedColumnTypes[i]) {
             throw BinderException(stringFormat("Column `{}` type mismatch. Expected {} but got {}.",
@@ -222,7 +222,7 @@ void Binder::validateNumColumns(uint32_t expectedNumber, uint32_t detectedNumber
 void Binder::sniffFiles(const common::ReaderConfig& readerConfig,
     std::vector<std::string>& columnNames,
     std::vector<std::unique_ptr<common::LogicalType>>& columnTypes) {
-    assert(readerConfig.getNumFiles() > 0);
+    KU_ASSERT(readerConfig.getNumFiles() > 0);
     sniffFile(readerConfig, 0, columnNames, columnTypes);
     for (auto i = 1; i < readerConfig.getNumFiles(); ++i) {
         std::vector<std::string> tmpColumnNames;

src/binder/bind/copy/bind_copy_rdf_graph.cpp

@@ -29,7 +29,7 @@ std::unique_ptr<BoundStatement> Binder::bindCopyRdfNodeFrom(
             std::make_unique<RdfReaderConfig>(RdfReaderMode::RESOURCE, nullptr /* index */);
         columns.push_back(createVariable(columnName, stringType));
     } else {
-        assert(tableSchema->tableName.ends_with(rdf::LITERAL_TABLE_SUFFIX));
+        KU_ASSERT(tableSchema->tableName.ends_with(rdf::LITERAL_TABLE_SUFFIX));
         containsSerial = true;
         readerConfig->columnTypes.push_back(RdfVariantType::getType());
         readerConfig->rdfReaderConfig =

src/binder/bind_expression/bind_function_expression.cpp

@@ -191,7 +191,7 @@ std::shared_ptr<Expression> ExpressionBinder::bindInternalIDExpression(
     if (ExpressionUtil::isRelVariable(*expression)) {
         return bindNodeOrRelPropertyExpression(*expression, InternalKeyword::ID);
     }
-    assert(expression->dataType.getPhysicalType() == PhysicalTypeID::STRUCT);
+    KU_ASSERT(expression->dataType.getPhysicalType() == PhysicalTypeID::STRUCT);
     auto stringValue =
         std::make_unique<Value>(LogicalType{LogicalTypeID::STRING}, InternalKeyword::ID);
     return bindScalarFunctionExpression(

src/binder/bind_expression/bind_property_expression.cpp

@@ -52,7 +52,7 @@ expression_vector ExpressionBinder::bindRelPropertyStarExpression(const Expressi
 
 expression_vector ExpressionBinder::bindStructPropertyStarExpression(
     std::shared_ptr<Expression> child) {
-    assert(child->getDataType().getLogicalTypeID() == LogicalTypeID::STRUCT);
+    KU_ASSERT(child->getDataType().getLogicalTypeID() == LogicalTypeID::STRUCT);
     expression_vector result;
     auto childType = child->getDataType();
     for (auto field : StructType::getFields(&childType)) {

src/binder/expression_binder.cpp

@@ -133,7 +133,7 @@ void ExpressionBinder::resolveAnyDataType(Expression& expression, const LogicalT
     if (expression.expressionType == PARAMETER) { // expression is parameter
         ((ParameterExpression&)expression).setDataType(targetType);
     } else { // expression is null literal
-        assert(expression.expressionType == LITERAL);
+        KU_ASSERT(expression.expressionType == LITERAL);
         ((LiteralExpression&)expression).setDataType(targetType);
     }
 }

src/binder/expression_visitor.cpp

@@ -115,7 +115,7 @@ bool ExpressionVisitor::satisfyAny(
 
 std::unordered_set<std::string> ExpressionCollector::getDependentVariableNames(
     const std::shared_ptr<Expression>& expression) {
-    assert(expressions.empty());
+    KU_ASSERT(expressions.empty());
     collectExpressionsInternal(expression, [&](const Expression& expression) {
         return expression.expressionType == ExpressionType::PROPERTY ||
                expression.expressionType == ExpressionType::VARIABLE;
@@ -126,7 +126,7 @@ std::unordered_set<std::string> ExpressionCollector::getDependentVariableNames(
             auto property = (PropertyExpression*)expr.get();
             result.insert(property->getVariableName());
         } else {
-            assert(expr->expressionType == ExpressionType::VARIABLE);
+            KU_ASSERT(expr->expressionType == ExpressionType::VARIABLE);
             result.insert(expr->getUniqueName());
         }
     }

src/binder/query/query_graph.cpp

@@ -155,7 +155,7 @@ void QueryGraph::addQueryNode(std::shared_ptr<NodeExpression> queryNode) {
 }
 
 void QueryGraph::addQueryRel(std::shared_ptr<RelExpression> queryRel) {
-    assert(!containsQueryRel(queryRel->getUniqueName()));
+    KU_ASSERT(!containsQueryRel(queryRel->getUniqueName()));
     queryRelNameToPosMap.insert({queryRel->getUniqueName(), queryRels.size()});
     queryRels.push_back(queryRel);
 }
@@ -273,7 +273,7 @@ bool PropertyKeyValCollection::hasKeyVal(
 
 expression_pair PropertyKeyValCollection::getKeyVal(
     std::shared_ptr<Expression> variable, const std::string& propertyName) const {
-    assert(hasKeyVal(variable, propertyName));
+    KU_ASSERT(hasKeyVal(variable, propertyName));
     return propertyKeyValMap.at(variable).at(propertyName);
 }
 

src/catalog/catalog_content.cpp

@@ -137,7 +137,7 @@ std::vector<TableSchema*> CatalogContent::getTableSchemas(
     const std::vector<table_id_t>& tableIDs) const {
     std::vector<TableSchema*> result;
     for (auto tableID : tableIDs) {
-        assert(tableSchemas.contains(tableID));
+        KU_ASSERT(tableSchemas.contains(tableID));
         result.push_back(tableSchemas.at(tableID).get());
     }
     return result;

src/common/CMakeLists.txt

@@ -9,7 +9,6 @@ add_subdirectory(vector)
 
 add_library(kuzu_common
         OBJECT
-        assert.cpp
         rel_direction.cpp
         expression_type.cpp
         file_utils.cpp

src/common/arrow/arrow_row_batch.cpp

@@ -50,7 +50,7 @@ template<>
 void ArrowRowBatch::templateInitializeVector<LogicalTypeID::VAR_LIST>(
     ArrowVector* vector, const main::DataTypeInfo& typeInfo, std::int64_t capacity) {
     initializeNullBits(vector->validity, capacity);
-    assert(typeInfo.childrenTypesInfo.size() == 1);
+    KU_ASSERT(typeInfo.childrenTypesInfo.size() == 1);
     auto childTypeInfo = typeInfo.childrenTypesInfo[0].get();
     // Initialize offsets and child buffer.
     vector->data.reserve((capacity + 1) * sizeof(std::uint32_t));

src/common/data_chunk/data_chunk.cpp

@@ -5,7 +5,7 @@ namespace common {
 
 void DataChunk::insert(uint32_t pos, std::shared_ptr<ValueVector> valueVector) {
     valueVector->setState(state);
-    assert(valueVectors.size() > pos);
+    KU_ASSERT(valueVectors.size() > pos);
     valueVectors[pos] = std::move(valueVector);
 }
 

src/common/in_mem_overflow_buffer.cpp

@@ -4,7 +4,7 @@ namespace kuzu {
 namespace common {
 
 uint8_t* InMemOverflowBuffer::allocateSpace(uint64_t size) {
-    assert(size <= BufferPoolConstants::PAGE_256KB_SIZE);
+    KU_ASSERT(size <= BufferPoolConstants::PAGE_256KB_SIZE);
     if (requireNewBlock(size)) {
         allocateNewBlock();
     }