Skip to content

Commit

Permalink
Avoid `error: invalid application of 'sizeof' to an incomplete type '…
Browse files Browse the repository at this point in the history
…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<rocksdb::FragmentedRangeTombstoneList>::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<rocksdb::FragmentedRangeTombstoneList>::~unique_ptr' requested here
   25 |   std::unique_ptr<FragmentedRangeTombstoneList> 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).
  • Loading branch information
mpatou-openai committed Jan 22, 2025
1 parent adb9f1a commit 674f6ce
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 11 deletions.
17 changes: 8 additions & 9 deletions db/range_tombstone_fragmenter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<FragmentedRangeTombstoneList> tombstones = nullptr;
// readers will first check this bool to avoid
std::atomic<bool> initialized = false;
};

struct FragmentedRangeTombstoneList {
public:
Expand Down Expand Up @@ -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<FragmentedRangeTombstoneList> tombstones = nullptr;
// readers will first check this bool to avoid
std::atomic<bool> initialized = false;
};


// FragmentedRangeTombstoneIterator converts an InternalIterator of a range-del
// meta block into an iterator over non-overlapping tombstone fragments. The
Expand Down
60 changes: 59 additions & 1 deletion table/block_based/block_based_table_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,64 @@ class BlockBasedTableBuilder::BlockBasedTablePropertiesCollector
bool prefix_filtering_;
};

namespace {
template <typename T>
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
Expand Down Expand Up @@ -355,7 +413,7 @@ struct BlockBasedTableBuilder::Rep {

std::vector<std::unique_ptr<InternalTblPropColl>> table_properties_collectors;

std::unique_ptr<ParallelCompressionRep> pc_rep;
HackUniquePtr<ParallelCompressionRep> pc_rep;
BlockCreateContext create_context;

// The size of the "tail" part of a SST file. "Tail" refers to
Expand Down
6 changes: 6 additions & 0 deletions util/xxhash.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <stddef.h>
Expand All @@ -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(); }

/*!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 674f6ce

Please sign in to comment.