cmake: enable Wall

As pointed out in #2536, -Wall is not enabled. -Wextra will follow in a separate PR. The main changes needed are: - Consistent use of either struct/class for a symbol, not a mix. - Don't call `std::move()` on temporaries, the compiler will elide the copy anyway. - Constructor initialization order should be the same as the member order. It turns out that C++ will always initialize the fields in the member order, regardless of the initialization list ordering. - Mark functions in header files as inline. Inline static doesn't really make sense it seems, and Apple mistakenly warns about it. Inline functions already have no linkage.
kuzudb · Dec 4, 2023 · 47a50c1 · 47a50c1
1 parent 97b78f8
commit 47a50c1
Show file tree

Hide file tree

Showing 47 changed files with 120 additions and 128 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -108,6 +108,9 @@ if(CMAKE_BUILD_TYPE MATCHES Release)
         set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O3")
     endif()
 endif()
+if(NOT MSVC)
+    add_compile_options(-Wall)
+endif()
 
 if(${ENABLE_THREAD_SANITIZER})
     if(MSVC)

diff --git a/src/binder/bind/bind_copy.cpp b/src/binder/bind/bind_copy.cpp
@@ -40,7 +40,7 @@ std::unique_ptr<BoundStatement> Binder::bindCopyToClause(const Statement& statem
     }
     auto csvConfig = bindParsingOptions(copyToStatement.getParsingOptionsRef());
     return std::make_unique<BoundCopyTo>(boundFilePath, fileType, std::move(columnNames),
-        std::move(columnTypes), std::move(query), std::move(csvConfig->option.copy()));
+        std::move(columnTypes), std::move(query), csvConfig->option.copy());
 }
 
 // As a temporary constraint, we require npy files loaded with COPY FROM BY COLUMN keyword.

diff --git a/src/common/arrow/arrow_converter.cpp b/src/common/arrow/arrow_converter.cpp
@@ -191,7 +191,7 @@ std::unique_ptr<ArrowSchema> ArrowConverter::toArrowSchema(
 
     outSchema->private_data = rootHolder.release();
     outSchema->release = releaseArrowSchema;
-    return std::move(outSchema);
+    return outSchema;
 }
 
 void ArrowConverter::toArrowArray(

diff --git a/src/common/arrow/arrow_row_batch.cpp b/src/common/arrow/arrow_row_batch.cpp
@@ -187,7 +187,7 @@ std::unique_ptr<ArrowVector> ArrowRowBatch::createVector(
         KU_UNREACHABLE;
     }
     }
-    return std::move(result);
+    return result;
 }
 
 static void getBitPosition(std::int64_t pos, std::int64_t& bytePos, std::int64_t& bitOffset) {
@@ -580,7 +580,7 @@ static std::unique_ptr<ArrowArray> createArrayFromVector(ArrowVector& vector) {
         result->n_buffers++;
         result->buffers[1] = vector.data.data();
     }
-    return std::move(result);
+    return result;
 }
 
 template<LogicalTypeID DT>

diff --git a/src/common/types/blob.cpp b/src/common/types/blob.cpp
@@ -48,10 +48,10 @@ uint64_t Blob::fromString(const char* str, uint64_t length, uint8_t* resultBuffe
     for (auto i = 0u; i < length; i++) {
         if (str[i] == '\\') {
             validateHexCode(reinterpret_cast<const uint8_t*>(str), length, i);
-            auto firstByte =
-                HexFormatConstants::HEX_MAP[str[i + HexFormatConstants::FIRST_BYTE_POS]];
-            auto secondByte =
-                HexFormatConstants::HEX_MAP[str[i + HexFormatConstants::SECOND_BYTES_POS]];
+            auto firstByte = HexFormatConstants::HEX_MAP[(
+                unsigned char)str[i + HexFormatConstants::FIRST_BYTE_POS]];
+            auto secondByte = HexFormatConstants::HEX_MAP[(
+                unsigned char)str[i + HexFormatConstants::SECOND_BYTES_POS]];
             resultBuffer[resultPos++] =
                 (firstByte << HexFormatConstants::NUM_BYTES_TO_SHIFT_FOR_FIRST_BYTE) + secondByte;
             i += HexFormatConstants::LENGTH - 1;

diff --git a/src/expression_evaluator/case_evaluator.cpp b/src/expression_evaluator/case_evaluator.cpp
@@ -73,7 +73,7 @@ std::unique_ptr<ExpressionEvaluator> CaseExpressionEvaluator::clone() {
         clonedAlternativeEvaluators.push_back(alternative->clone());
     }
     return make_unique<CaseExpressionEvaluator>(
-        expression, std::move(clonedAlternativeEvaluators), std::move(elseEvaluator->clone()));
+        expression, std::move(clonedAlternativeEvaluators), elseEvaluator->clone());
 }
 
 void CaseExpressionEvaluator::resolveResultVector(
@@ -101,7 +101,7 @@ void CaseExpressionEvaluator::fillEntry(sel_t resultPos, const ValueVector& then
     } else {
         if (thenVector.dataType.getLogicalTypeID() == LogicalTypeID::VAR_LIST) {
             auto srcListEntry = thenVector.getValue<list_entry_t>(thenPos);
-            list_entry_t resultEntry = ListVector::addList(resultVector.get(), srcListEntry.size);
+            ListVector::addList(resultVector.get(), srcListEntry.size);
             resultVector->copyFromVectorData(resultPos, &thenVector, thenPos);
         } else {
             auto val = thenVector.getValue<T>(thenPos);

diff --git a/src/function/cast/cast_fixed_list.cpp b/src/function/cast/cast_fixed_list.cpp
@@ -30,7 +30,6 @@ bool CastFixedListHelper::containsListToFixedList(
                         dstType->toString())};
             }
 
-            auto result = false;
             std::vector<struct_field_idx_t> fields;
             for (auto i = 0u; i < srcFieldTypes.size(); i++) {
                 if (containsListToFixedList(srcFieldTypes[i], dstFieldTypes[i])) {

diff --git a/src/include/binder/binder.h b/src/include/binder/binder.h
@@ -19,17 +19,17 @@ class ClientContext;
 }
 
 namespace function {
-class TableFunction;
+struct TableFunction;
 }
 
 namespace binder {
 
-class BoundInsertInfo;
-class BoundSetPropertyInfo;
-class BoundDeleteInfo;
+struct BoundInsertInfo;
+struct BoundSetPropertyInfo;
+struct BoundDeleteInfo;
 class BoundWithClause;
 class BoundReturnClause;
-class BoundFileScanInfo;
+struct BoundFileScanInfo;
 
 // BinderScope keeps track of expressions in scope and their aliases. We maintain the order of
 // expressions in

diff --git a/src/include/binder/expression_binder.h b/src/include/binder/expression_binder.h
@@ -11,7 +11,7 @@ class Value;
 namespace binder {
 
 class Binder;
-class CaseAlternative;
+struct CaseAlternative;
 
 class ExpressionBinder {
     friend class Binder;

diff --git a/src/include/binder/query/updating_clause/bound_delete_info.h b/src/include/binder/query/updating_clause/bound_delete_info.h
@@ -14,11 +14,11 @@ struct BoundDeleteInfo {
 
     BoundDeleteInfo(UpdateTableType updateTableType, std::shared_ptr<Expression> nodeOrRel,
         common::DeleteClauseType deleteClauseType)
-        : updateTableType{updateTableType}, nodeOrRel{std::move(nodeOrRel)},
-          deleteClauseType{deleteClauseType} {}
+        : deleteClauseType{deleteClauseType}, updateTableType{updateTableType}, nodeOrRel{std::move(
+                                                                                    nodeOrRel)} {}
     BoundDeleteInfo(const BoundDeleteInfo& other)
-        : updateTableType{other.updateTableType}, nodeOrRel{other.nodeOrRel},
-          deleteClauseType{other.deleteClauseType} {}
+        : deleteClauseType{other.deleteClauseType},
+          updateTableType{other.updateTableType}, nodeOrRel{other.nodeOrRel} {}
 
     inline std::unique_ptr<BoundDeleteInfo> copy() {
         return std::make_unique<BoundDeleteInfo>(*this);

diff --git a/src/include/common/utils.h b/src/include/common/utils.h
@@ -33,7 +33,7 @@ class BitmaskUtils {
     }
 };
 
-static uint64_t nextPowerOfTwo(uint64_t v) {
+inline uint64_t nextPowerOfTwo(uint64_t v) {
     v--;
     v |= v >> 1;
     v |= v >> 2;
@@ -45,7 +45,7 @@ static uint64_t nextPowerOfTwo(uint64_t v) {
     return v;
 }
 
-static bool isLittleEndian() {
+inline bool isLittleEndian() {
     // Little endian arch stores the least significant value in the lower bytes.
     int testNumber = 1;
     return *(uint8_t*)&testNumber == 1;

diff --git a/src/include/function/aggregate_function.h b/src/include/function/aggregate_function.h
@@ -9,8 +9,6 @@
 namespace kuzu {
 namespace function {
 
-class AggregateFunction;
-
 struct AggregateState {
     virtual inline uint32_t getStateSize() const = 0;
     virtual void moveResultToVector(common::ValueVector* outputVector, uint64_t pos) = 0;
@@ -37,10 +35,10 @@ struct AggregateFunction final : public BaseScalarFunction {
         scalar_bind_func bindFunc = nullptr, param_rewrite_function_t paramRewriteFunc = nullptr)
         : BaseScalarFunction{FunctionType::AGGREGATE, std::move(name), std::move(parameterTypeIDs),
               returnTypeID, std::move(bindFunc)},
-          initializeFunc{std::move(initializeFunc)}, updateAllFunc{std::move(updateAllFunc)},
-          updatePosFunc{std::move(updatePosFunc)}, combineFunc{std::move(combineFunc)},
-          finalizeFunc{std::move(finalizeFunc)}, isDistinct{isDistinct}, paramRewriteFunc{std::move(
-                                                                             paramRewriteFunc)} {
+          isDistinct{isDistinct}, initializeFunc{std::move(initializeFunc)},
+          updateAllFunc{std::move(updateAllFunc)}, updatePosFunc{std::move(updatePosFunc)},
+          combineFunc{std::move(combineFunc)}, finalizeFunc{std::move(finalizeFunc)},
+          paramRewriteFunc{std::move(paramRewriteFunc)} {
         initialNullAggregateState = createInitialNullAggregateState();
     }
 

diff --git a/src/include/function/cast/functions/cast_string_non_nested_functions.h b/src/include/function/cast/functions/cast_string_non_nested_functions.h
@@ -53,7 +53,7 @@ void castStringToBool(const char* input, uint64_t len, bool& result);
 // cast to numerical values
 // TODO(Kebing): support exponent + decimal
 template<typename T, bool NEGATIVE, bool ALLOW_EXPONENT = false, class OP>
-static bool integerCastLoop(const char* input, uint64_t len, T& result) {
+inline bool integerCastLoop(const char* input, uint64_t len, T& result) {
     int64_t start_pos = 0;
     if (NEGATIVE) {
         start_pos = 1;
@@ -75,7 +75,7 @@ static bool integerCastLoop(const char* input, uint64_t len, T& result) {
 }
 
 template<typename T, bool IS_SIGNED = true, class OP = IntegerCastOperation>
-static bool tryIntegerCast(const char* input, uint64_t& len, T& result) {
+inline bool tryIntegerCast(const char* input, uint64_t& len, T& result) {
     StringUtils::removeCStringWhiteSpaces(input, len);
     if (len == 0) {
         return false;
@@ -159,7 +159,7 @@ struct Int128CastOperation {
     }
 };
 
-static bool trySimpleInt128Cast(const char* input, uint64_t len, int128_t& result) {
+inline bool trySimpleInt128Cast(const char* input, uint64_t len, int128_t& result) {
     Int128CastData data{};
     data.result = 0;
     if (tryIntegerCast<Int128CastData, true, Int128CastOperation>(input, len, data)) {
@@ -169,15 +169,15 @@ static bool trySimpleInt128Cast(const char* input, uint64_t len, int128_t& resul
     return false;
 }
 
-static void simpleInt128Cast(const char* input, uint64_t len, int128_t& result) {
+inline void simpleInt128Cast(const char* input, uint64_t len, int128_t& result) {
     if (!trySimpleInt128Cast(input, len, result)) {
         throw ConversionException(
             stringFormat("Cast failed. {} is not within INT128 range.", std::string{input, len}));
     }
 }
 
 template<typename T, bool IS_SIGNED = true>
-static bool trySimpleIntegerCast(const char* input, uint64_t len, T& result) {
+inline bool trySimpleIntegerCast(const char* input, uint64_t len, T& result) {
     IntegerCastData<T> data;
     data.result = 0;
     if (tryIntegerCast<IntegerCastData<T>, IS_SIGNED>(input, len, data)) {
@@ -188,7 +188,7 @@ static bool trySimpleIntegerCast(const char* input, uint64_t len, T& result) {
 }
 
 template<class T, bool IS_SIGNED = true>
-static void simpleIntegerCast(
+inline void simpleIntegerCast(
     const char* input, uint64_t len, T& result, LogicalTypeID typeID = LogicalTypeID::ANY) {
     if (!trySimpleIntegerCast<T, IS_SIGNED>(input, len, result)) {
         throw ConversionException(stringFormat("Cast failed. {} is not in {} range.",
@@ -197,7 +197,7 @@ static void simpleIntegerCast(
 }
 
 template<class T>
-static bool tryDoubleCast(const char* input, uint64_t len, T& result) {
+inline bool tryDoubleCast(const char* input, uint64_t len, T& result) {
     StringUtils::removeCStringWhiteSpaces(input, len);
     if (len == 0) {
         return false;
@@ -217,7 +217,7 @@ static bool tryDoubleCast(const char* input, uint64_t len, T& result) {
 }
 
 template<class T>
-static void doubleCast(
+inline void doubleCast(
     const char* input, uint64_t len, T& result, LogicalTypeID typeID = LogicalTypeID::ANY) {
     if (!tryDoubleCast<T>(input, len, result)) {
         throw ConversionException(stringFormat("Cast failed. {} is not in {} range.",

diff --git a/src/include/function/cast/functions/numeric_cast.h b/src/include/function/cast/functions/numeric_cast.h
@@ -9,7 +9,7 @@ namespace kuzu {
 namespace function {
 
 template<class SRC, class DST>
-static bool tryCastWithOverflowCheck(SRC value, DST& result) {
+inline bool tryCastWithOverflowCheck(SRC value, DST& result) {
     if (NumericLimits<SRC>::isSigned() != NumericLimits<DST>::isSigned()) {
         if (NumericLimits<SRC>::isSigned()) {
             if (NumericLimits<SRC>::digits() > NumericLimits<DST>::digits()) {
@@ -53,7 +53,7 @@ static bool tryCastWithOverflowCheck(SRC value, DST& result) {
 }
 
 template<class SRC, class T>
-bool tryCastWithOverflowCheckFloat(SRC value, T& result, SRC min, SRC max) {
+inline bool tryCastWithOverflowCheckFloat(SRC value, T& result, SRC min, SRC max) {
     if (!(value >= min && value < max)) {
         return false;
     }
@@ -63,99 +63,99 @@ bool tryCastWithOverflowCheckFloat(SRC value, T& result, SRC min, SRC max) {
 }
 
 template<>
-bool tryCastWithOverflowCheck(float value, int8_t& result) {
+inline bool tryCastWithOverflowCheck(float value, int8_t& result) {
     return tryCastWithOverflowCheckFloat<float, int8_t>(value, result, -128.0f, 128.0f);
 }
 
 template<>
-bool tryCastWithOverflowCheck(float value, int16_t& result) {
+inline bool tryCastWithOverflowCheck(float value, int16_t& result) {
     return tryCastWithOverflowCheckFloat<float, int16_t>(value, result, -32768.0f, 32768.0f);
 }
 
 template<>
-bool tryCastWithOverflowCheck(float value, int32_t& result) {
+inline bool tryCastWithOverflowCheck(float value, int32_t& result) {
     return tryCastWithOverflowCheckFloat<float, int32_t>(
         value, result, -2147483648.0f, 2147483648.0f);
 }
 
 template<>
-bool tryCastWithOverflowCheck(float value, int64_t& result) {
+inline bool tryCastWithOverflowCheck(float value, int64_t& result) {
     return tryCastWithOverflowCheckFloat<float, int64_t>(
         value, result, -9223372036854775808.0f, 9223372036854775808.0f);
 }
 
 template<>
-bool tryCastWithOverflowCheck(float value, uint8_t& result) {
+inline bool tryCastWithOverflowCheck(float value, uint8_t& result) {
     return tryCastWithOverflowCheckFloat<float, uint8_t>(value, result, 0.0f, 256.0f);
 }
 
 template<>
-bool tryCastWithOverflowCheck(float value, uint16_t& result) {
+inline bool tryCastWithOverflowCheck(float value, uint16_t& result) {
     return tryCastWithOverflowCheckFloat<float, uint16_t>(value, result, 0.0f, 65536.0f);
 }
 
 template<>
-bool tryCastWithOverflowCheck(float value, uint32_t& result) {
+inline bool tryCastWithOverflowCheck(float value, uint32_t& result) {
     return tryCastWithOverflowCheckFloat<float, uint32_t>(value, result, 0.0f, 4294967296.0f);
 }
 
 template<>
-bool tryCastWithOverflowCheck(float value, uint64_t& result) {
+inline bool tryCastWithOverflowCheck(float value, uint64_t& result) {
     return tryCastWithOverflowCheckFloat<float, uint64_t>(
         value, result, 0.0f, 18446744073709551616.0f);
 }
 
 template<>
-bool tryCastWithOverflowCheck(double value, int8_t& result) {
+inline bool tryCastWithOverflowCheck(double value, int8_t& result) {
     return tryCastWithOverflowCheckFloat<double, int8_t>(value, result, -128.0, 128.0);
 }
 
 template<>
-bool tryCastWithOverflowCheck(double value, int16_t& result) {
+inline bool tryCastWithOverflowCheck(double value, int16_t& result) {
     return tryCastWithOverflowCheckFloat<double, int16_t>(value, result, -32768.0, 32768.0);
 }
 
 template<>
-bool tryCastWithOverflowCheck(double value, int32_t& result) {
+inline bool tryCastWithOverflowCheck(double value, int32_t& result) {
     return tryCastWithOverflowCheckFloat<double, int32_t>(
         value, result, -2147483648.0, 2147483648.0);
 }
 
 template<>
-bool tryCastWithOverflowCheck(double value, int64_t& result) {
+inline bool tryCastWithOverflowCheck(double value, int64_t& result) {
     return tryCastWithOverflowCheckFloat<double, int64_t>(
         value, result, -9223372036854775808.0, 9223372036854775808.0);
 }
 
 template<>
-bool tryCastWithOverflowCheck(double value, uint8_t& result) {
+inline bool tryCastWithOverflowCheck(double value, uint8_t& result) {
     return tryCastWithOverflowCheckFloat<double, uint8_t>(value, result, 0.0, 256.0);
 }
 
 template<>
-bool tryCastWithOverflowCheck(double value, uint16_t& result) {
+inline bool tryCastWithOverflowCheck(double value, uint16_t& result) {
     return tryCastWithOverflowCheckFloat<double, uint16_t>(value, result, 0.0, 65536.0);
 }
 
 template<>
-bool tryCastWithOverflowCheck(double value, uint32_t& result) {
+inline bool tryCastWithOverflowCheck(double value, uint32_t& result) {
     return tryCastWithOverflowCheckFloat<double, uint32_t>(value, result, 0.0, 4294967296.0);
 }
 
 template<>
-bool tryCastWithOverflowCheck(double value, uint64_t& result) {
+inline bool tryCastWithOverflowCheck(double value, uint64_t& result) {
     return tryCastWithOverflowCheckFloat<double, uint64_t>(
         value, result, 0.0, 18446744073709551615.0);
 }
 
 template<>
-bool tryCastWithOverflowCheck(float input, double& result) {
+inline bool tryCastWithOverflowCheck(float input, double& result) {
     result = double(input);
     return true;
 }
 
 template<>
-bool tryCastWithOverflowCheck(double input, float& result) {
+inline bool tryCastWithOverflowCheck(double input, float& result) {
     result = float(input);
     return true;
 }