Skip to content

Commit

Permalink
Parameterize ExternalRef::StorageWhole by TargetRefT<Arg> instead…
Browse files Browse the repository at this point in the history
… of by

`Arg&&`.

The implementation of `StorageWhole` depends only on the target type, not on
its initializer, because the object needs to be constructed unconditionaly for
conversion to `absl::string_view`; the `StorageWhole` keeps the constructed
object rather than its initializer.

While this is inconsistent with `ExternalRef::StorageSubstr`, this allows
to skip the details of `MakerTypeFor` and pass only the target type if the
`StorageWhole` is passed explicitly — which is very rare, hence regular code
is unaffected.

PiperOrigin-RevId: 715287089
  • Loading branch information
QrczakMK committed Jan 14, 2025
1 parent 16617f2 commit 092fb4d
Showing 1 changed file with 28 additions and 22 deletions.
50 changes: 28 additions & 22 deletions riegeli/base/external_ref_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -1818,21 +1818,19 @@ class ExternalRef {
ABSL_ATTRIBUTE_NO_UNIQUE_ADDRESS TemporaryStorage<T> temporary_storage_;
};

template <typename Arg, typename Enable = void>
template <typename T, typename Enable = void>
struct StorageWholeImpl;

template <typename Arg>
struct StorageWholeImpl<Arg,
std::enable_if_t<!HasCallOperator<
absl::remove_cvref_t<TargetRefT<Arg>>>::value>> {
using type = StorageWholeWithoutCallOperator<TargetRefT<Arg>>;
template <typename T>
struct StorageWholeImpl<
T, std::enable_if_t<!HasCallOperator<absl::remove_cvref_t<T>>::value>> {
using type = StorageWholeWithoutCallOperator<T>;
};

template <typename Arg>
template <typename T>
struct StorageWholeImpl<
Arg, std::enable_if_t<
HasCallOperator<absl::remove_cvref_t<TargetRefT<Arg>>>::value>> {
using type = StorageWholeWithCallOperator<TargetRefT<Arg>>;
T, std::enable_if_t<HasCallOperator<absl::remove_cvref_t<T>>::value>> {
using type = StorageWholeWithCallOperator<T>;
};

template <typename Arg, typename Enable = void>
Expand All @@ -1856,8 +1854,8 @@ class ExternalRef {
// The type of the `storage` parameter for the constructor and
// `ExternalRef::From()` which take an external object supporting
// `riegeli::ToStringView()`.
template <typename Arg>
using StorageWhole = typename StorageWholeImpl<Arg>::type;
template <typename T>
using StorageWhole = typename StorageWholeImpl<T>::type;

// The type of the `storage` parameter for the constructor and
// `ExternalRef::From()` which take an external object and its substring.
Expand All @@ -1874,9 +1872,8 @@ class ExternalRef {
std::enable_if_t<SupportsExternalRefWhole<TargetRefT<Arg>>::value,
int> = 0>
/*implicit*/ ExternalRef(Arg&& arg ABSL_ATTRIBUTE_LIFETIME_BOUND,
StorageWhole<Arg&&>&& storage
ABSL_ATTRIBUTE_LIFETIME_BOUND =
StorageWhole<Arg&&>())
StorageWhole<TargetRefT<Arg>>&& storage
ABSL_ATTRIBUTE_LIFETIME_BOUND = {})
: storage_(&storage) {
storage.Initialize(std::forward<Arg>(arg));
}
Expand All @@ -1887,13 +1884,18 @@ class ExternalRef {
// `substr` must be owned by the object if it gets created or moved.
//
// `storage` must outlive usages of the returned `ExternalRef`.
//
// The object is not created if an initializer is passed rather than an
// already constructed object, the object type does not use the call operator,
// and only `absl::string_view` turns out to be needed. Hence `StorageSubstr`
// is parameterized by `Arg&&` rather than `TargetRefT<Arg>`, so that it can
// keep the original initializer.
template <typename Arg,
std::enable_if_t<SupportsExternalRefSubstr<TargetRefT<Arg>>::value,
int> = 0>
explicit ExternalRef(
Arg&& arg ABSL_ATTRIBUTE_LIFETIME_BOUND, absl::string_view substr,
StorageSubstr<Arg&&>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND =
StorageSubstr<Arg&&>())
StorageSubstr<Arg&&>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND = {})
: storage_(&storage) {
storage.Initialize(std::forward<Arg>(arg), substr);
}
Expand All @@ -1908,21 +1910,25 @@ class ExternalRef {
typename Arg,
std::enable_if_t<SupportsToStringView<TargetRefT<Arg>>::value, int> = 0>
static ExternalRef From(Arg&& arg ABSL_ATTRIBUTE_LIFETIME_BOUND,
StorageWhole<Arg&&>&& storage
ABSL_ATTRIBUTE_LIFETIME_BOUND =
StorageWhole<Arg&&>()) {
StorageWhole<TargetRefT<Arg>>&& storage
ABSL_ATTRIBUTE_LIFETIME_BOUND = {}) {
storage.Initialize(std::forward<Arg>(arg));
return ExternalRef(&storage);
}

// Like `ExternalRef` constructor, but `RiegeliSupportsExternalRef()` is not
// needed. The caller is responsible for using an appropriate type of the
// external object.
//
// The object is not created if an initializer is passed rather than an
// already constructed object, the object type does not use the call operator,
// and only `absl::string_view` turns out to be needed. Hence `StorageSubstr`
// is parameterized by `Arg&&` rather than `TargetRefT<Arg>`, so that it can
// keep the original initializer.
template <typename Arg>
static ExternalRef From(
Arg&& arg ABSL_ATTRIBUTE_LIFETIME_BOUND, absl::string_view substr,
StorageSubstr<Arg&&>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND =
StorageSubstr<Arg&&>()) {
StorageSubstr<Arg&&>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND = {}) {
storage.Initialize(std::forward<Arg>(arg), substr);
return ExternalRef(&storage);
}
Expand Down

0 comments on commit 092fb4d

Please sign in to comment.