Skip to content

Commit

Permalink
Reorder overloads to put handling of ExternalRef directly after
Browse files Browse the repository at this point in the history 
`absl::string_view` or `BytesRef`.

Planned changes to `ExternalRef` make it more natural to put them together,
followed by disambiguating overloads for other types if needed.

Also, if `!std::is_convertible_v<Src&&, BytesRef>>` then there is no need to
ensure that `!SupportsExternalRefWhole<Src>::value`. This is always implied.

PiperOrigin-RevId: 720154616
  • Loading branch information
@QrczakMK
QrczakMK committed Jan 27, 2025
1 parent 2cfa81c commit 1b70dd0
Show file tree
Hide file tree
Showing 55 changed files with 487 additions and 491 deletions.
36 changes: 18 additions & 18 deletions riegeli/base/chain_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,13 @@ class Chain : public WithCompare<Chain> {

// Converts from a string-like type.
explicit Chain(BytesRef src);
explicit Chain(Block src);
explicit Chain(const absl::Cord& src);
explicit Chain(absl::Cord&& src);
explicit Chain(ExternalRef src);
template <typename Src,
std::enable_if_t<SupportsExternalRefWhole<Src>::value, int> = 0>
explicit Chain(Src&& src);
explicit Chain(Block src);
explicit Chain(const absl::Cord& src);
explicit Chain(absl::Cord&& src);

Chain(const Chain& that);
Chain& operator=(const Chain& that);
Expand All @@ -217,13 +217,13 @@ class Chain : public WithCompare<Chain> {
// constructing a temporary `Chain` and moving from it.
ABSL_ATTRIBUTE_REINITIALIZES void Reset();
ABSL_ATTRIBUTE_REINITIALIZES void Reset(BytesRef src);
ABSL_ATTRIBUTE_REINITIALIZES void Reset(Block src);
ABSL_ATTRIBUTE_REINITIALIZES void Reset(const absl::Cord& src);
ABSL_ATTRIBUTE_REINITIALIZES void Reset(absl::Cord&& src);
ABSL_ATTRIBUTE_REINITIALIZES void Reset(ExternalRef src);
template <typename Src,
std::enable_if_t<SupportsExternalRefWhole<Src>::value, int> = 0>
ABSL_ATTRIBUTE_REINITIALIZES void Reset(Src&& src);
ABSL_ATTRIBUTE_REINITIALIZES void Reset(Block src);
ABSL_ATTRIBUTE_REINITIALIZES void Reset(const absl::Cord& src);
ABSL_ATTRIBUTE_REINITIALIZES void Reset(absl::Cord&& src);

// Removes all data.
ABSL_ATTRIBUTE_REINITIALIZES void Clear();
Expand Down Expand Up @@ -304,12 +304,6 @@ class Chain : public WithCompare<Chain> {

// Appends/prepends a string-like type.
void Append(BytesRef src, Options options = Options());
void Append(const Chain& src, Options options = Options());
void Append(Chain&& src, Options options = Options());
void Append(const Block& src, Options options = Options());
void Append(Block&& src, Options options = Options());
void Append(const absl::Cord& src, Options options = Options());
void Append(absl::Cord&& src, Options options = Options());
void Append(ExternalRef src);
void Append(ExternalRef src, Options options);
template <typename Src,
Expand All @@ -318,13 +312,13 @@ class Chain : public WithCompare<Chain> {
template <typename Src,
std::enable_if_t<SupportsExternalRefWhole<Src>::value, int> = 0>
void Append(Src&& src, Options options);
void Append(const Chain& src, Options options = Options());
void Append(Chain&& src, Options options = Options());
void Append(const Block& src, Options options = Options());
void Append(Block&& src, Options options = Options());
void Append(const absl::Cord& src, Options options = Options());
void Append(absl::Cord&& src, Options options = Options());
void Prepend(BytesRef src, Options options = Options());
void Prepend(const Chain& src, Options options = Options());
void Prepend(Chain&& src, Options options = Options());
void Prepend(const Block& src, Options options = Options());
void Prepend(Block&& src, Options options = Options());
void Prepend(const absl::Cord& src, Options options = Options());
void Prepend(absl::Cord&& src, Options options = Options());
void Prepend(ExternalRef src);
void Prepend(ExternalRef src, Options options);
template <typename Src,
Expand All @@ -333,6 +327,12 @@ class Chain : public WithCompare<Chain> {
template <typename Src,
std::enable_if_t<SupportsExternalRefWhole<Src>::value, int> = 0>
void Prepend(Src&& src, Options options);
void Prepend(const Chain& src, Options options = Options());
void Prepend(Chain&& src, Options options = Options());
void Prepend(const Block& src, Options options = Options());
void Prepend(Block&& src, Options options = Options());
void Prepend(const absl::Cord& src, Options options = Options());
void Prepend(absl::Cord&& src, Options options = Options());

// `AppendFrom(iter, length)` is equivalent to
// `Append(absl::Cord::AdvanceAndRead(&iter, length))` but more efficient.
Expand Down
8 changes: 4 additions & 4 deletions riegeli/base/chain_details.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -961,10 +961,6 @@ constexpr size_t Chain::kExternalAllocatedSize() {

inline Chain::Chain(BytesRef src) { Initialize(src); }

inline Chain::Chain(Block src) {
if (src.raw_block() != nullptr) Initialize(std::move(src));
}

inline Chain::Chain(ExternalRef src) { std::move(src).InitializeTo(*this); }

template <typename Src,
Expand All @@ -973,6 +969,10 @@ inline Chain::Chain(Src&& src) {
ExternalRef(std::forward<Src>(src)).InitializeTo(*this);
}

inline Chain::Chain(Block src) {
if (src.raw_block() != nullptr) Initialize(std::move(src));
}

inline Chain::Chain(Chain&& that) noexcept
: size_(std::exchange(that.size_, 0)) {
// Use `std::memcpy()` instead of copy constructor to silence
Expand Down
14 changes: 7 additions & 7 deletions riegeli/bytes/backward_writer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ bool BackwardWriter::WriteSlow(absl::string_view src) {
return true;
}

bool BackwardWriter::WriteSlow(ExternalRef src) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), src.size())
<< "Failed precondition of BackwardWriter::WriteSlow(ExternalRef): "
"enough space available, use Write(ExternalRef) instead";
return Write(absl::string_view(src));
}

bool BackwardWriter::WriteSlow(const Chain& src) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), src.size())
<< "Failed precondition of BackwardWriter::WriteSlow(Chain): "
Expand Down Expand Up @@ -136,13 +143,6 @@ bool BackwardWriter::WriteSlow(absl::Cord&& src) {
return WriteSlow(src);
}

bool BackwardWriter::WriteSlow(ExternalRef src) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), src.size())
<< "Failed precondition of BackwardWriter::WriteSlow(ExternalRef): "
"enough space available, use Write(ExternalRef) instead";
return Write(absl::string_view(src));
}

bool BackwardWriter::WriteSlow(ByteFill src) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), src.size())
<< "Failed precondition of BackwardWriter::WriteSlow(ByteFill): "
Expand Down
64 changes: 31 additions & 33 deletions riegeli/bytes/backward_writer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,14 @@ class BackwardWriter : public Object {
bool Write(BytesRef src);
ABSL_ATTRIBUTE_ALWAYS_INLINE
bool Write(const char* src) { return Write(absl::string_view(src)); }
bool Write(const Chain& src);
bool Write(Chain&& src);
bool Write(const absl::Cord& src);
bool Write(absl::Cord&& src);
bool Write(ExternalRef src);
template <typename Src,
std::enable_if_t<SupportsExternalRefWhole<Src>::value, int> = 0>
bool Write(Src&& src);
bool Write(const Chain& src);
bool Write(Chain&& src);
bool Write(const absl::Cord& src);
bool Write(absl::Cord&& src);
bool Write(ByteFill src);

// Writes a stringified value to the buffer and/or the destination.
Expand Down Expand Up @@ -201,7 +201,6 @@ class BackwardWriter : public Object {
absl::negation<std::is_convertible<Src&&, BytesRef>>,
absl::negation<std::is_convertible<Src&&, const Chain&>>,
absl::negation<std::is_convertible<Src&&, const absl::Cord&>>,
absl::negation<SupportsExternalRefWhole<Src>>,
absl::negation<std::is_convertible<Src&&, ByteFill>>>::value,
int> = 0>
bool Write(Src&& src);
Expand Down Expand Up @@ -376,8 +375,8 @@ class BackwardWriter : public Object {
// By default:
// * `WriteSlow(absl::string_view)` and `WriteSlow(ByteFill)` are
// implemented in terms of `PushSlow()`
// * `WriteSlow(const Chain&)`, `WriteSlow(const absl::Cord&)`, and
// `WriteSlow(ExternalRef)` are implemented in terms of
// * `WriteSlow(ExternalRef)`, `WriteSlow(const Chain&)`, and
// `WriteSlow(const absl::Cord&)` are implemented in terms of
// `WriteSlow(absl::string_view)`
// * `WriteSlow(Chain&&)` is implemented in terms of
// `WriteSlow(const Chain&)`
Expand All @@ -387,16 +386,16 @@ class BackwardWriter : public Object {
// Precondition for `WriteSlow(absl::string_view)`:
// `available() < src.size()`
//
// Precondition for `WriteSlow(const Chain&)`, `WriteSlow(Chain&&)`,
// `WriteSlow(const absl::Cord&)`, `WriteSlow(absl::Cord&&),
// `WriteSlow(ExternalRef)`, and `WriteSlow(ByteFill)`:
// Precondition for `WriteSlow(ExternalRef)`, `WriteSlow(const Chain&)`,
// `WriteSlow(Chain&&)`, `WriteSlow(const absl::Cord&)`,
// `WriteSlow(absl::Cord&&), and `WriteSlow(ByteFill)`:
// `UnsignedMin(available(), kMaxBytesToCopy) < src.size()`
virtual bool WriteSlow(absl::string_view src);
virtual bool WriteSlow(ExternalRef src);
virtual bool WriteSlow(const Chain& src);
virtual bool WriteSlow(Chain&& src);
virtual bool WriteSlow(const absl::Cord& src);
virtual bool WriteSlow(absl::Cord&& src);
virtual bool WriteSlow(ExternalRef src);
virtual bool WriteSlow(ByteFill src);

// Implementation of `Flush()`, except that the parameter is not defaulted,
Expand Down Expand Up @@ -591,6 +590,27 @@ inline bool BackwardWriter::Write(BytesRef src) {
return WriteSlow(src);
}

inline bool BackwardWriter::Write(ExternalRef src) {
if (ABSL_PREDICT_TRUE(available() >= src.size() &&
src.size() <= kMaxBytesToCopy)) {
// `std::memcpy(nullptr, _, 0)` and `std::memcpy(_, nullptr, 0)` are
// undefined.
if (ABSL_PREDICT_TRUE(!src.empty())) {
move_cursor(src.size());
std::memcpy(cursor(), src.data(), src.size());
}
return true;
}
AssertInitialized(cursor(), start_to_cursor());
return WriteSlow(std::move(src));
}

template <typename Src,
std::enable_if_t<SupportsExternalRefWhole<Src>::value, int>>
inline bool BackwardWriter::Write(Src&& src) {
return Write(ExternalRef(std::forward<Src>(src)));
}

inline bool BackwardWriter::Write(const Chain& src) {
#ifdef MEMORY_SANITIZER
for (const absl::string_view fragment : src.blocks()) {
Expand Down Expand Up @@ -655,27 +675,6 @@ inline bool BackwardWriter::Write(absl::Cord&& src) {
return WriteSlow(std::move(src));
}

inline bool BackwardWriter::Write(ExternalRef src) {
if (ABSL_PREDICT_TRUE(available() >= src.size() &&
src.size() <= kMaxBytesToCopy)) {
// `std::memcpy(nullptr, _, 0)` and `std::memcpy(_, nullptr, 0)` are
// undefined.
if (ABSL_PREDICT_TRUE(!src.empty())) {
move_cursor(src.size());
std::memcpy(cursor(), src.data(), src.size());
}
return true;
}
AssertInitialized(cursor(), start_to_cursor());
return WriteSlow(std::move(src));
}

template <typename Src,
std::enable_if_t<SupportsExternalRefWhole<Src>::value, int>>
inline bool BackwardWriter::Write(Src&& src) {
return Write(ExternalRef(std::forward<Src>(src)));
}

inline bool BackwardWriter::Write(ByteFill src) {
if (ABSL_PREDICT_TRUE(available() >= src.size() &&
src.size() <= kMaxBytesToCopy)) {
Expand Down Expand Up @@ -745,7 +744,6 @@ template <typename Src,
absl::negation<std::is_convertible<Src&&, BytesRef>>,
absl::negation<std::is_convertible<Src&&, const Chain&>>,
absl::negation<std::is_convertible<Src&&, const absl::Cord&>>,
absl::negation<SupportsExternalRefWhole<Src>>,
absl::negation<std::is_convertible<Src&&, ByteFill>>>::value,
int>>
inline bool BackwardWriter::Write(Src&& src) {
Expand Down
38 changes: 19 additions & 19 deletions riegeli/bytes/chain_backward_writer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,25 @@ bool ChainBackwardWriterBase::PushSlow(size_t min_length,
return true;
}

bool ChainBackwardWriterBase::WriteSlow(ExternalRef src) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), src.size())
<< "Failed precondition of BackwardWriter::WriteSlow(ExternalRef): "
"enough space available, use Write(ExternalRef) instead";
if (ABSL_PREDICT_FALSE(!ok())) return false;
Chain& dest = *DestChain();
RIEGELI_ASSERT_EQ(limit_pos(), dest.size())
<< "ChainBackwardWriter destination changed unexpectedly";
SyncBuffer(dest);
if (ABSL_PREDICT_FALSE(src.size() > std::numeric_limits<size_t>::max() -
IntCast<size_t>(start_pos()))) {
return FailOverflow();
}
move_start_pos(src.size());
dest.Prepend(std::move(src), options_);
MakeBuffer(dest);
return true;
}

bool ChainBackwardWriterBase::WriteSlow(const Chain& src) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), src.size())
<< "Failed precondition of BackwardWriter::WriteSlow(Chain): "
Expand Down Expand Up @@ -175,25 +194,6 @@ bool ChainBackwardWriterBase::WriteSlow(absl::Cord&& src) {
return true;
}

bool ChainBackwardWriterBase::WriteSlow(ExternalRef src) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), src.size())
<< "Failed precondition of BackwardWriter::WriteSlow(ExternalRef): "
"enough space available, use Write(ExternalRef) instead";
if (ABSL_PREDICT_FALSE(!ok())) return false;
Chain& dest = *DestChain();
RIEGELI_ASSERT_EQ(limit_pos(), dest.size())
<< "ChainBackwardWriter destination changed unexpectedly";
SyncBuffer(dest);
if (ABSL_PREDICT_FALSE(src.size() > std::numeric_limits<size_t>::max() -
IntCast<size_t>(start_pos()))) {
return FailOverflow();
}
move_start_pos(src.size());
dest.Prepend(std::move(src), options_);
MakeBuffer(dest);
return true;
}

bool ChainBackwardWriterBase::FlushImpl(FlushType flush_type) {
if (ABSL_PREDICT_FALSE(!ok())) return false;
Chain& dest = *DestChain();
Expand Down
2 changes: 1 addition & 1 deletion riegeli/bytes/chain_backward_writer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,11 @@ class ChainBackwardWriterBase : public BackwardWriter {
void SetWriteSizeHintImpl(absl::optional<Position> write_size_hint) override;
bool PushSlow(size_t min_length, size_t recommended_length) override;
using BackwardWriter::WriteSlow;
bool WriteSlow(ExternalRef src) override;
bool WriteSlow(const Chain& src) override;
bool WriteSlow(Chain&& src) override;
bool WriteSlow(const absl::Cord& src) override;
bool WriteSlow(absl::Cord&& src) override;
bool WriteSlow(ExternalRef src) override;
bool WriteSlow(ByteFill src) override;
bool FlushImpl(FlushType flush_type) override;
bool TruncateImpl(Position new_size) override;
Expand Down
Loading

0 comments on commit 1b70dd0

Please sign in to comment.