Main commit proxies

.clang-tidy

@@ -6,12 +6,16 @@ Checks: >
   bugprone-chained-comparison,
   bugprone-copy-constructor-init,
   bugprone-dangling-handle,
+  bugprone-fold-init-type,
+  bugprone-forwarding-reference-overload,
   bugprone-implicit-widening-of-multiplication-result,
   bugprone-inaccurate-erase,
   bugprone-infinite-loop,
+  bugprone-integer-division,
   bugprone-macro-repeated-side-effects,
   bugprone-misplaced-widening-cast,
   bugprone-move-forwarding-reference,
+  bugprone-posix-return,
   bugprone-redundant-branch-condition,
   bugprone-return-const-ref-from-parameter,
   bugprone-shared-ptr-array-mismatch,
@@ -29,6 +33,7 @@ Checks: >
   bugprone-swapped-arguments,
   bugprone-too-small-loop-variable,
   bugprone-undefined-memory-manipulation,
+  bugprone-unhandled-self-assignment,
   bugprone-unique-ptr-array-mismatch,
   bugprone-use-after-move,
   bugprone-virtual-near-miss,

.github/workflows/tidy.yml

@@ -46,6 +46,7 @@ jobs:
             fdboptions                  \
             ProtocolVersion             \
             fdb_c_generated             \
+            fdb_c_options               \
             fdb-java
 
           # all protobuf headers

CMakeLists.txt

@@ -40,6 +40,9 @@ if("${PROJECT_SOURCE_DIR}" STREQUAL "${PROJECT_BINARY_DIR}")
   message(FATAL_ERROR "In-source builds are forbidden")
 endif()
 
+set(FDB_RELEASE           OFF CACHE BOOL "This is a building of a final release")
+set(FDB_RELEASE_CANDIDATE OFF CACHE BOOL "This is a building of a release candidate")
+
 set(OPEN_FOR_IDE
     OFF
     CACHE BOOL "Open this in an IDE (won't compile/link)")
@@ -54,23 +57,18 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
         Debug
         CACHE STRING "Choose the type of build" FORCE)
   else()
-    message(STATUS "Setting build type to 'Release' as none was specified")
+    message(STATUS "Setting build type to 'Release'")
     set(CMAKE_BUILD_TYPE
         Release
         CACHE STRING "Choose the type of build" FORCE)
-    set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release"
-                                                 "MinSizeRel" "RelWithDebInfo")
   endif()
+  set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release"
+                                               "MinSizeRel" "RelWithDebInfo")
 endif()
 
 set(EXECUTABLE_OUTPUT_PATH ${PROJECT_BINARY_DIR}/bin)
 set(LIBRARY_OUTPUT_PATH ${PROJECT_BINARY_DIR}/lib)
 
-option(USE_SCCACHE "Use sccache if found" ON)
-if(USE_SCCACHE)
-  find_package(sccache)
-endif()
-
 option(FDB_MEMORY_TRACKER "Compile in the sampled per-call-site memory tracker (flow/MemoryTracker)" ON)
 if(NOT FDB_MEMORY_TRACKER)
   message(STATUS "Building FoundationDB with the memory tracker compiled out")

bindings/c/CMakeLists.txt

@@ -354,15 +354,17 @@ if(NOT WIN32)
       )
     endforeach()
 
-    add_python_venv_test(NAME fdb_c_upgrade_to_future_version
-      COMMAND python -m fdb_test_runner.upgrade_test
-      --build-dir ${CMAKE_BINARY_DIR}
-      --test-file ${CMAKE_SOURCE_DIR}/bindings/c/test/apitester/tests/upgrade/MixedApiWorkloadMultiThr.toml
-      --upgrade-path "${FDB_CURRENT_VERSION}" "${FDB_FUTURE_VERSION}" "${FDB_CURRENT_VERSION}"
-      --process-number 3
-      )
+    if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+      add_python_venv_test(NAME fdb_c_upgrade_to_future_version
+        COMMAND python -m fdb_test_runner.upgrade_test
+        --build-dir ${CMAKE_BINARY_DIR}
+        --test-file ${CMAKE_SOURCE_DIR}/bindings/c/test/apitester/tests/upgrade/MixedApiWorkloadMultiThr.toml
+        --upgrade-path "${FDB_CURRENT_VERSION}" "${FDB_FUTURE_VERSION}" "${FDB_CURRENT_VERSION}"
+        --process-number 3
+        )
+    endif()
 
-    if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64" AND NOT USE_SANITIZER)
+    if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64" AND NOT USE_SANITIZER)
       add_python_venv_test(NAME fdb_c_client_config_tests
         COMMAND python ${CMAKE_CURRENT_SOURCE_DIR}/test/fdb_c_client_config_tests.py
         --build-dir ${CMAKE_BINARY_DIR}

bindings/c/fdb_c.cpp

@@ -95,14 +95,42 @@ FDBKeyRange copyNativeCdcKeyRange(Arena& arena, KeyRangeRef source) {
 	return FDBKeyRange{ begin.begin(), begin.size(), end.begin(), end.size() };
 }
 
+std::vector<KeyRange> copyNativeCdcRanges(FDBKeyRange const* ranges, int rangeCount) {
+	if (rangeCount <= 0 || rangeCount > NATIVE_CDC_MAX_RANGES || ranges == nullptr) {
+		throw client_invalid_operation();
+	}
+	std::vector<KeyRange> result;
+	result.reserve(rangeCount);
+	for (int i = 0; i < rangeCount; ++i) {
+		auto const& range = ranges[i];
+		if (range.begin_key_length < 0 || range.end_key_length < 0 ||
+		    (range.begin_key_length > 0 && range.begin_key == nullptr) ||
+		    (range.end_key_length > 0 && range.end_key == nullptr)) {
+			throw client_invalid_operation();
+		}
+		KeyRef begin(range.begin_key, range.begin_key_length);
+		KeyRef end(range.end_key, range.end_key_length);
+		if (begin >= end) {
+			throw client_invalid_operation();
+		}
+		result.emplace_back(KeyRangeRef(begin, end));
+	}
+	return result;
+}
+
 CNativeCdcStreamInfoArray makeCNativeCdcStreamInfoArray(std::vector<NativeCdcStreamInfo> const& source) {
 	CNativeCdcStreamInfoArray result;
 	result.streams.reserve(result.arena, source.size());
 	for (auto const& stream : source) {
 		FDBCdcStreamInfo cStream;
 		cStream.name = copyNativeCdcKey(result.arena, stream.name);
 		cStream.stream_id = stream.streamId;
-		cStream.key_range = copyNativeCdcKeyRange(result.arena, stream.keys);
+		cStream.range_count = stream.ranges.size();
+		auto* ranges = new (result.arena) FDBKeyRange[stream.ranges.size()];
+		for (int i = 0; i < cStream.range_count; ++i) {
+			ranges[i] = copyNativeCdcKeyRange(result.arena, stream.ranges[i]);
+		}
+		cStream.ranges = ranges;
 		cStream.min_version = stream.minVersion;
 		result.streams.push_back(result.arena, cStream);
 	}
@@ -565,17 +593,13 @@ extern "C" DLLEXPORT fdb_error_t fdb_database_create_transaction(FDBDatabase* d,
 extern "C" DLLEXPORT FDBFuture* fdb_database_register_cdc_stream(FDBDatabase* db,
                                                                  uint8_t const* name,
                                                                  int name_length,
-                                                                 uint8_t const* begin_key,
-                                                                 int begin_key_length,
-                                                                 uint8_t const* end_key,
-                                                                 int end_key_length) {
+                                                                 FDBKeyRange const* ranges,
+                                                                 int range_count) {
 	RETURN_FUTURE_ON_ERROR(
 	    CDCStreamId,
-	    return (FDBFuture*)(DB(db)
-	                            ->registerNativeCdcStream(
-	                                KeyRef(name, name_length),
-	                                KeyRangeRef(KeyRef(begin_key, begin_key_length), KeyRef(end_key, end_key_length)))
-	                            .extractPtr()););
+	    if (name_length <= 0 || name == nullptr) { throw client_invalid_operation(); } auto rangesCopy =
+	        copyNativeCdcRanges(ranges, range_count);
+	    return (FDBFuture*)(DB(db)->registerNativeCdcStream(KeyRef(name, name_length), rangesCopy).extractPtr()););
 }
 
 extern "C" DLLEXPORT FDBFuture* fdb_database_remove_cdc_stream(FDBDatabase* db, uint8_t const* name, int name_length) {

bindings/c/foundationdb/CppWorkload.h

@@ -25,6 +25,7 @@
 #include <vector>
 #include <functional>
 #include <memory>
+#include <type_traits>
 
 #ifndef DLLEXPORT
 #if defined(_MSC_VER)
@@ -81,7 +82,8 @@ class GenericPromise {
 	std::shared_ptr<FDBPromise> impl;
 
 public:
-	template <class Ptr>
+	template <class Ptr,
+	          typename std::enable_if<std::is_constructible<std::shared_ptr<FDBPromise>, Ptr&&>::value, int>::type = 0>
 	explicit GenericPromise(Ptr&& impl) : impl(std::forward<Ptr>(impl)) {}
 	void send(T val) { impl->send(&val); }
 };

bindings/c/foundationdb/fdb_c.h

@@ -213,7 +213,8 @@ typedef enum {
 typedef struct cdc_stream_info {
 	FDBKey name;
 	uint64_t stream_id;
-	FDBKeyRange key_range;
+	const FDBKeyRange* ranges;
+	int range_count;
 	int64_t min_version;
 } FDBCdcStreamInfo;
 
@@ -437,10 +438,8 @@ DLLEXPORT WARN_UNUSED_RESULT fdb_error_t fdb_database_create_transaction(FDBData
 DLLEXPORT WARN_UNUSED_RESULT FDBFuture* fdb_database_register_cdc_stream(FDBDatabase* db,
                                                                          uint8_t const* name,
                                                                          int name_length,
-                                                                         uint8_t const* begin_key,
-                                                                         int begin_key_length,
-                                                                         uint8_t const* end_key,
-                                                                         int end_key_length);
+                                                                         FDBKeyRange const* ranges,
+                                                                         int range_count);
 
 DLLEXPORT WARN_UNUSED_RESULT FDBFuture* fdb_database_remove_cdc_stream(FDBDatabase* db,
                                                                        uint8_t const* name,

bindings/c/test/fdb_api.hpp

@@ -139,6 +139,8 @@ CharsRef toCharsRef(const std::optional<StringLike<Char>>& s) noexcept {
 
 [[maybe_unused]] constexpr const bool OverflowCheck = false;
 
+// clang-tidy does not recognize size() through intSize(); call-site suppressions mark
+// byte buffers whose lengths are passed explicitly to the C API or KeySelector.
 inline int intSize(size_t size) {
 	if constexpr (OverflowCheck) {
 		if (size > static_cast<size_t>(std::numeric_limits<int>::max()))
@@ -278,6 +280,7 @@ inline int maxApiVersion() {
 namespace network {
 
 inline Error setOptionNothrow(FDBNetworkOption option, BytesRef str) noexcept {
+	// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
 	return Error(native::fdb_network_set_option(option, str.data(), intSize(str.size())));
 }
 
@@ -345,7 +348,7 @@ class Result {
 	friend class Transaction;
 	std::shared_ptr<native::FDBResult> r;
 
-	Result(native::FDBResult* result) {
+	explicit Result(native::FDBResult* result) {
 		if (result)
 			r = std::shared_ptr<native::FDBResult>(result, &native::fdb_result_destroy);
 	}
@@ -377,7 +380,7 @@ class Future {
 	friend std::hash<Future>;
 	std::shared_ptr<native::FDBFuture> f;
 
-	Future(native::FDBFuture* future) {
+	explicit(false) Future(native::FDBFuture* future) {
 		if (future)
 			f = std::shared_ptr<native::FDBFuture>(future, &native::fdb_future_destroy);
 	}
@@ -473,7 +476,7 @@ class TypedFuture : public Future {
 	using Future::get;
 	using Future::getNothrow;
 	using Future::then;
-	TypedFuture(const Future& f) noexcept : Future(f) {}
+	explicit TypedFuture(const Future& f) noexcept : Future(f) {}
 
 public:
 	using ContainedType = typename VarTraits::Type;
@@ -500,18 +503,22 @@ struct KeySelector {
 namespace key_select {
 
 inline KeySelector firstGreaterThan(KeyRef key, int offset = 0) {
+	// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
 	return KeySelector{ FDB_KEYSEL_FIRST_GREATER_THAN(key.data(), intSize(key.size())) + offset };
 }
 
 inline KeySelector firstGreaterOrEqual(KeyRef key, int offset = 0) {
+	// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
 	return KeySelector{ FDB_KEYSEL_FIRST_GREATER_OR_EQUAL(key.data(), intSize(key.size())) + offset };
 }
 
 inline KeySelector lastLessThan(KeyRef key, int offset = 0) {
+	// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
 	return KeySelector{ FDB_KEYSEL_LAST_LESS_THAN(key.data(), intSize(key.size())) + offset };
 }
 
 inline KeySelector lastLessOrEqual(KeyRef key, int offset = 0) {
+	// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
 	return KeySelector{ FDB_KEYSEL_LAST_LESS_OR_EQUAL(key.data(), intSize(key.size())) + offset };
 }
 
@@ -549,6 +556,7 @@ class Transaction {
 	}
 
 	Error setOptionNothrow(FDBTransactionOption option, BytesRef str) noexcept {
+		// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
 		return Error(native::fdb_transaction_set_option(tr.get(), option, str.data(), intSize(str.size())));
 	}
 
@@ -600,6 +608,7 @@ class Transaction {
 	}
 
 	TypedFuture<future_var::ValueRef> get(KeyRef key, bool snapshot) {
+		// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
 		return native::fdb_transaction_get(tr.get(), key.data(), intSize(key.size()), snapshot);
 	}
 
@@ -631,6 +640,7 @@ class Transaction {
 	}
 
 	TypedFuture<future_var::None> watch(KeyRef key) {
+		// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
 		return native::fdb_transaction_watch(tr.get(), key.data(), intSize(key.size()));
 	}
 
@@ -643,26 +653,36 @@ class Transaction {
 	void cancel() { return native::fdb_transaction_cancel(tr.get()); }
 
 	void set(KeyRef key, ValueRef value) {
+		// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
 		native::fdb_transaction_set(tr.get(), key.data(), intSize(key.size()), value.data(), intSize(value.size()));
 	}
 
 	void atomicOp(KeyRef key, ValueRef param, FDBMutationType operationType) {
+		// NOLINTBEGIN(bugprone-suspicious-stringview-data-usage)
 		native::fdb_transaction_atomic_op(
 		    tr.get(), key.data(), intSize(key.size()), param.data(), intSize(param.size()), operationType);
+		// NOLINTEND(bugprone-suspicious-stringview-data-usage)
 	}
 
-	void clear(KeyRef key) { native::fdb_transaction_clear(tr.get(), key.data(), intSize(key.size())); }
+	void clear(KeyRef key) {
+		// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
+		native::fdb_transaction_clear(tr.get(), key.data(), intSize(key.size()));
+	}
 
 	void clearRange(KeyRef begin, KeyRef end) {
+		// NOLINTBEGIN(bugprone-suspicious-stringview-data-usage)
 		native::fdb_transaction_clear_range(
 		    tr.get(), begin.data(), intSize(begin.size()), end.data(), intSize(end.size()));
+		// NOLINTEND(bugprone-suspicious-stringview-data-usage)
 	}
 
 	void addConflictRange(KeyRef begin, KeyRef end, FDBConflictRangeType rangeType) {
+		// NOLINTBEGIN(bugprone-suspicious-stringview-data-usage)
 		if (auto err = Error(native::fdb_transaction_add_conflict_range(
 		        tr.get(), begin.data(), intSize(begin.size()), end.data(), intSize(end.size()), rangeType))) {
 			throwError("fdb_transaction_add_conflict_range returned error: ", err);
 		}
+		// NOLINTEND(bugprone-suspicious-stringview-data-usage)
 	}
 
 	void addReadConflictRange(KeyRef begin, KeyRef end) { addConflictRange(begin, end, FDB_CONFLICT_RANGE_TYPE_READ); }
@@ -686,7 +706,7 @@ class Database : public IDatabaseOps {
 public:
 	Database(const Database&) noexcept = default;
 	Database& operator=(const Database&) noexcept = default;
-	Database(const std::string& cluster_file_path) : db(nullptr) {
+	explicit Database(const std::string& cluster_file_path) : db(nullptr) {
 		auto db_raw = static_cast<native::FDBDatabase*>(nullptr);
 		if (auto err = Error(native::fdb_create_database(cluster_file_path.c_str(), &db_raw)))
 			throwError(fmt::format("Failed to create database with '{}': ", cluster_file_path), err);
@@ -708,6 +728,7 @@ class Database : public IDatabaseOps {
 	}
 
 	Error setOptionNothrow(FDBDatabaseOption option, BytesRef str) noexcept {
+		// NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
 		return Error(native::fdb_database_set_option(db.get(), option, str.data(), intSize(str.size())));
 	}
 

bindings/c/test/mako/admin_server.hpp

@@ -95,7 +95,7 @@ class AdminServer {
 	}
 
 public:
-	AdminServer(const Arguments& args)
+	explicit AdminServer(const Arguments& args)
 	  : args(args), server_pid(-1), pipe_to_server(boost::process::pipe()), pipe_to_client(boost::process::pipe()) {
 		start();
 	}

bindings/c/test/mako/async.cpp

@@ -101,15 +101,15 @@ void ResumableStateForRunWorkload::postNextTick() {
 
 void ResumableStateForRunWorkload::runOneTick() {
 	assert(iter != OpEnd);
+	auto f = Future{};
+	// to minimize context switch overhead, repeat immediately completed ops
+	// in a loop, not an async continuation.
+repeat_immediate_steps:
 	if (iter.step == 0 /* first step */)
 		prepareKeys(iter.op, key1, key2, args);
 	watch_step.start();
 	if (iter.step == 0)
 		watch_op = Stopwatch(watch_step.getStart());
-	auto f = Future{};
-	// to minimize context switch overhead, repeat immediately completed ops
-	// in a loop, not an async continuation.
-repeat_immediate_steps:
 	f = opTable[iter.op].stepFunction(iter.step)(tx, args, key1, key2, val);
 	if (!f) {
 		// immediately completed client-side ops: e.g. set, setrange, clear, clearrange, ...

bindings/c/test/mako/ddsketch.hpp

@@ -220,7 +220,7 @@ class DDSketch : public DDSketchBase<DDSketch<T>, T> {
 template <class T>
 class DDSketchSlow : public DDSketchBase<DDSketchSlow<T>, T> {
 public:
-	DDSketchSlow(double errorGuarantee = 0.1)
+	explicit DDSketchSlow(double errorGuarantee = 0.1)
 	  : DDSketchBase<DDSketchSlow<T>, T>(errorGuarantee), gamma((1.0 + errorGuarantee) / (1.0 - errorGuarantee)),
 	    logGamma(log(gamma)) {
 		offset = getIndex(1.0 / DDSketchBase<DDSketch<T>, T>::EPS) + 5;

bindings/c/test/mako/future.hpp

@@ -36,7 +36,7 @@ enum class FutureRC { OK, RETRY, ABORT };
 
 struct LogContext {
 	static constexpr const bool do_log = true;
-	LogContext(std::string_view step) noexcept : step(step), transaction_timeout_expected(false) {}
+	explicit LogContext(std::string_view step) noexcept : step(step), transaction_timeout_expected(false) {}
 	LogContext(std::string_view step, bool transaction_timeout_expected) noexcept
 	  : step(step), transaction_timeout_expected(transaction_timeout_expected) {}
 	std::string_view step;