From 674f6ce018197896f9c16cd5bfc1aba8f64c4788 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Wed, 22 Jan 2025 18:26:33 +0000 Subject: [PATCH] Avoid `error: invalid application of 'sizeof' to an incomplete type 'rocksdb:xxx'` when compiling with C++-23 When trying to compile with c++-23 I'm getting errors like: ``` /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:91:16: error: invalid application of 'sizeof' to an incomplete type 'rocksdb::FragmentedRangeTombstoneList' 91 | static_assert(sizeof(_Tp)>0, | ^~~~~~~~~~~ /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:398:4: note: in instantiation of member function 'std::default_delete::operator()' requested here 398 | get_deleter()(std::move(__ptr)); | ^ cpp/third-party/rocksdb-cloud/db/range_tombstone_fragmenter.h:25:62: note: in instantiation of member function 'std::unique_ptr::~unique_ptr' requested here 25 | std::unique_ptr tombstones = nullptr; | ^ cpp/third-party/rocksdb-cloud/db/range_tombstone_fragmenter.h:20:8: note: forward declaration of 'rocksdb::FragmentedRangeTombstoneList' 20 | struct FragmentedRangeTombstoneList; | ^ ``` With this fix declaration are reargenged to avoid this and when not possible we give explicit constructor/destructors and move them to a different place (ie. not in the headers). --- db/range_tombstone_fragmenter.h | 17 +++--- .../block_based/block_based_table_builder.cc | 60 ++++++++++++++++++- util/xxhash.h | 6 ++ .../write_batch_with_index_internal.cc | 1 + .../write_batch_with_index_internal.h | 2 +- 5 files changed, 75 insertions(+), 11 deletions(-) diff --git a/db/range_tombstone_fragmenter.h b/db/range_tombstone_fragmenter.h index 8bdbf03be97f..1d17c1ddf58e 100644 --- a/db/range_tombstone_fragmenter.h +++ b/db/range_tombstone_fragmenter.h @@ -17,15 +17,6 @@ #include "table/internal_iterator.h" namespace ROCKSDB_NAMESPACE { -struct FragmentedRangeTombstoneList; - -struct FragmentedRangeTombstoneListCache { - // ensure only the first reader needs to initialize l - std::mutex reader_mutex; - std::unique_ptr tombstones = nullptr; - // readers will first check this bool to avoid - std::atomic initialized = false; -}; struct FragmentedRangeTombstoneList { public: @@ -123,6 +114,14 @@ struct FragmentedRangeTombstoneList { uint64_t num_unfragmented_tombstones_; uint64_t total_tombstone_payload_bytes_; }; +struct FragmentedRangeTombstoneListCache { + // ensure only the first reader needs to initialize l + std::mutex reader_mutex; + std::unique_ptr tombstones = nullptr; + // readers will first check this bool to avoid + std::atomic initialized = false; +}; + // FragmentedRangeTombstoneIterator converts an InternalIterator of a range-del // meta block into an iterator over non-overlapping tombstone fragments. The diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index eb2ae4ddba9d..b013c3064783 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -259,6 +259,64 @@ class BlockBasedTableBuilder::BlockBasedTablePropertiesCollector bool prefix_filtering_; }; +namespace { +template +class HackUniquePtr { +private: + T* ptr; + +public: + // Constructor + explicit HackUniquePtr(T* p = nullptr) : ptr(p) {} + + // Destructor + ~HackUniquePtr() { + delete ptr; + } + + // Move constructor + HackUniquePtr(HackUniquePtr&& other) noexcept : ptr(other.ptr) { + other.ptr = nullptr; + } + + // Move assignment operator + HackUniquePtr& operator=(HackUniquePtr&& other) noexcept { + if (this != &other) { + delete ptr; + ptr = other.ptr; + other.ptr = nullptr; + } + return *this; + } + + // Delete copy constructor and copy assignment + HackUniquePtr(const HackUniquePtr&) = delete; + HackUniquePtr& operator=(const HackUniquePtr&) = delete; + + // Overloaded dereference and arrow operators + T& operator*() const { return *ptr; } + T* operator->() const { return ptr; } + + // Get the raw pointer + T* get() const { return ptr; } + + // Release ownership of the pointer + T* release() { + T* temp = ptr; + ptr = nullptr; + return temp; + } + + // Reset the unique pointer with a new raw pointer + void reset(T* newPtr = nullptr) { + delete ptr; + ptr = newPtr; + } +}; + +} + + struct BlockBasedTableBuilder::Rep { const ImmutableOptions ioptions; // BEGIN from MutableCFOptions @@ -355,7 +413,7 @@ struct BlockBasedTableBuilder::Rep { std::vector> table_properties_collectors; - std::unique_ptr pc_rep; + HackUniquePtr pc_rep; BlockCreateContext create_context; // The size of the "tail" part of a SST file. "Tail" refers to diff --git a/util/xxhash.h b/util/xxhash.h index bd31cfafbc4a..a3286fc54f51 100644 --- a/util/xxhash.h +++ b/util/xxhash.h @@ -2058,6 +2058,9 @@ static int XXH_isLittleEndian(void) #endif +#if defined (__cplusplus) +} +#endif #if defined(__STDC_VERSION__) && (__STDC_VERSION__ > 201710L) /* C23 and future versions have standard "unreachable()" */ # include @@ -2078,6 +2081,9 @@ static int XXH_isLittleEndian(void) # define XXH_UNREACHABLE() #endif +#if defined (__cplusplus) +extern "C" { +#endif #define XXH_ASSUME(c) if (!(c)) { XXH_UNREACHABLE(); } /*! diff --git a/utilities/write_batch_with_index/write_batch_with_index_internal.cc b/utilities/write_batch_with_index/write_batch_with_index_internal.cc index 84e30b7cc8ce..e7dcf8a36237 100644 --- a/utilities/write_batch_with_index/write_batch_with_index_internal.cc +++ b/utilities/write_batch_with_index/write_batch_with_index_internal.cc @@ -35,6 +35,7 @@ BaseDeltaIterator::BaseDeltaIterator(ColumnFamilyHandle* column_family, assert(delta_iterator_); assert(comparator_); } +inline BaseDeltaIterator::~BaseDeltaIterator() {}; bool BaseDeltaIterator::Valid() const { return status_.ok() ? (current_at_base_ ? BaseValid() : DeltaValid()) : false; diff --git a/utilities/write_batch_with_index/write_batch_with_index_internal.h b/utilities/write_batch_with_index/write_batch_with_index_internal.h index 163de2014d43..45c3716b3262 100644 --- a/utilities/write_batch_with_index/write_batch_with_index_internal.h +++ b/utilities/write_batch_with_index/write_batch_with_index_internal.h @@ -38,7 +38,7 @@ class BaseDeltaIterator : public Iterator { WBWIIteratorImpl* delta_iterator, const Comparator* comparator); - ~BaseDeltaIterator() override {} + ~BaseDeltaIterator() override; bool Valid() const override; void SeekToFirst() override;