From 81944c48d14da4882d5cb8bb48da2c38c130e092 Mon Sep 17 00:00:00 2001 From: Kaviraj Date: Mon, 15 Jun 2026 10:27:09 +0000 Subject: [PATCH] chore: avoid silent implicit type conversions follow up to https://github.com/ClickHouse/silk/pull/73 Added some compiler flags to avoid such silent converstions in the future. Played with different options and these combinations seems to be practical at the moment without much noise. Signed-off-by: Kaviraj --- CMakeLists.txt | 30 +++++++++++++++++++++ contrib/liburing-cmake/CMakeLists.txt | 2 +- src/fibers/cpu.cpp | 2 +- src/fibers/fiber.cpp | 6 ++--- src/util/benchmarks/bounded-queue-bench.cpp | 2 +- 5 files changed, 36 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2554589..fbc6e2c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -136,6 +136,36 @@ include(GoogleTest) include_directories(include) include_directories(src) +# Stricter implicit-conversion warnings for first-party code only. Placed after +# all contrib add_subdirectory() calls so third-party sources (librseq, +# libbacktrace, liburing, etc.) keep compiling under their own assumptions; +# add_compile_options only affects targets created later in this scope. Catches +# silent value-altering conversions: integer narrowing (uint64_t -> uint32_t), +# float-to-int truncation, and float-precision loss. +# None of these are enabled by -Wall/-Wextra. +# Third-party headers we include (e.g. liburing) are marked SYSTEM so their +# inline functions do not trip these here. +add_compile_options( + # Integer narrowing. Already includes the -Wshorten-64-to-32 subgroup, so + # that 64->32 flag would be redundant here. + -Wimplicit-int-conversion + + # float/double -> int truncation. + -Wfloat-conversion + + # double -> float narrowing, and int -> float precision loss + # (-Wimplicit-float-conversion includes the -Wimplicit-int-float-conversion + # subgroup). Also bit noisy + # -Wimplicit-float-conversion + + # Signed <-> unsigned changes. Correct in principle but very noisy on + # existing code; enable once the tree is clean. + # -Wsign-conversion + + # This catches all types of implicit conversations. But currently too noisy + # -Wconversion +) + add_subdirectory(src/util) add_subdirectory(src/fibers) add_subdirectory(src/gdb) diff --git a/contrib/liburing-cmake/CMakeLists.txt b/contrib/liburing-cmake/CMakeLists.txt index 1757104..b9e0822 100644 --- a/contrib/liburing-cmake/CMakeLists.txt +++ b/contrib/liburing-cmake/CMakeLists.txt @@ -77,7 +77,7 @@ add_library(liburing STATIC # LIBURING_BIN must come before LIBURING_SRC/src/include so the generated # liburing/compat.h and liburing/io_uring_version.h shadow the absent originals. -target_include_directories(liburing PUBLIC ${LIBURING_BIN} ${LIBURING_SRC}/src/include) +target_include_directories(liburing SYSTEM PUBLIC ${LIBURING_BIN} ${LIBURING_SRC}/src/include) target_compile_definitions(liburing PRIVATE _GNU_SOURCE diff --git a/src/fibers/cpu.cpp b/src/fibers/cpu.cpp index 8529580..12f6b5b 100644 --- a/src/fibers/cpu.cpp +++ b/src/fibers/cpu.cpp @@ -36,7 +36,7 @@ static int readSysfsUint32(const char * path, uint32_t * out) noexcept buf[n] = '\0'; char * end; - uint32_t val = ::strtoul(buf, &end, 10); + uint32_t val = static_cast(::strtoul(buf, &end, 10)); if (end == buf) { return EINVAL; diff --git a/src/fibers/fiber.cpp b/src/fibers/fiber.cpp index abf8dfe..0f7aff6 100644 --- a/src/fibers/fiber.cpp +++ b/src/fibers/fiber.cpp @@ -637,7 +637,7 @@ struct FiberScheduler::ProcessorState void FiberScheduler::ProcessorState::initialize(uint32_t cpu) noexcept { SILK_ASSERT(cpu < INVALID_PROCESSOR_NUMBER); - number = cpu; + number = static_cast(cpu); readyQueue.initialize(options.readyQueueCapacity); @@ -1041,7 +1041,7 @@ void FiberScheduler::initialize(const Options * userOptions) noexcept if (CPU_ISSET(cpu, &processCpuSet)) { ProcessorState * processor = &scheduler->processorState[cpu]; - processor->number = cpu; + processor->number = static_cast(cpu); } } @@ -1266,7 +1266,7 @@ void FiberScheduler::enqueueReady(Fiber * fiber) noexcept { if (fiber->processorNumber == INVALID_PROCESSOR_NUMBER) { - fiber->processorNumber = getCurrentProcessor(); + fiber->processorNumber = static_cast(getCurrentProcessor()); } ProcessorState * processor = &scheduler->processorState[fiber->processorNumber]; diff --git a/src/util/benchmarks/bounded-queue-bench.cpp b/src/util/benchmarks/bounded-queue-bench.cpp index f6babfd..0844c4c 100644 --- a/src/util/benchmarks/bounded-queue-bench.cpp +++ b/src/util/benchmarks/bounded-queue-bench.cpp @@ -38,7 +38,7 @@ class BoundedQueueBench : public benchmark::Fixture { for (uint64_t i = 0; i < CAPACITY; ++i) { - bool b = queue.enqueue(i); + bool b = queue.enqueue(static_cast(i)); SILK_ASSERT(b); } }