Skip to content

Commit ccefe1e

Browse files
Abseil Teamcopybara-github
authored andcommitted
Lift restriction on using EBCO[1] for nested CompressedTuples.
The current implementation of CompressedTuple explicitly disallows EBCO for cases where CompressedTuples are nested. This is because the implentation for a tuple with EBCO-compatible element T inherits from Storage<T, I>, where I is the index of T in the tuple, and CompressedTuple<T, CompressedTuple<T>> would inherit twice from Storage<T, 0>, leading to ambiguity. This CL lifts the restriction by tagging Storage with a tag unique to a concrete CompressedTuple type. In the above example, the storage classes for the two T's will be `Storage<T, 0, StorageTag<T, CompressedTuple<T>>` and `Storage<T, 0, StorageTag<T>>`, respectively. [1] https://en.cppreference.com/w/cpp/language/ebo.html PiperOrigin-RevId: 766677179 Change-Id: I4f0169fadc7466a5d799ae1168b2ed1f195b363e
1 parent 169c953 commit ccefe1e

File tree

2 files changed

+36
-24
lines changed

2 files changed

+36
-24
lines changed

absl/container/internal/compressed_tuple.h

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -64,24 +64,24 @@ struct Elem<CompressedTuple<B...>, I>
6464
template <typename D, size_t I>
6565
using ElemT = typename Elem<D, I>::type;
6666

67-
// We can't use EBCO on other CompressedTuples because that would mean that we
68-
// derive from multiple Storage<> instantiations with the same I parameter,
69-
// and potentially from multiple identical Storage<> instantiations. So anytime
70-
// we use type inheritance rather than encapsulation, we mark
71-
// CompressedTupleImpl, to make this easy to detect.
72-
struct uses_inheritance {};
7367

7468
template <typename T>
7569
constexpr bool ShouldUseBase() {
7670
return std::is_class<T>::value && std::is_empty<T>::value &&
77-
!std::is_final<T>::value &&
78-
!std::is_base_of<uses_inheritance, T>::value;
71+
!std::is_final<T>::value;
7972
}
8073

74+
// Tag type used to disambiguate Storage types for different CompresseedTuples.
75+
// Without it, CompressedTuple<T, CompressedTuple<T>> would inherit from
76+
// Storage<T, 0> twice.
77+
template <typename... Ts>
78+
struct StorageTag;
79+
8180
// The storage class provides two specializations:
8281
// - For empty classes, it stores T as a base class.
8382
// - For everything else, it stores T as a member.
84-
template <typename T, size_t I, bool UseBase = ShouldUseBase<T>()>
83+
// Tag should be set to StorageTag<Ts...>.
84+
template <typename T, size_t I, typename Tag, bool UseBase = ShouldUseBase<T>()>
8585
struct Storage {
8686
T value;
8787
constexpr Storage() = default;
@@ -94,8 +94,8 @@ struct Storage {
9494
constexpr T&& get() && { return std::move(*this).value; }
9595
};
9696

97-
template <typename T, size_t I>
98-
struct ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC Storage<T, I, true> : T {
97+
template <typename T, size_t I, typename Tag>
98+
struct ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC Storage<T, I, Tag, true> : T {
9999
constexpr Storage() = default;
100100

101101
template <typename V>
@@ -111,30 +111,35 @@ template <typename D, typename I, bool ShouldAnyUseBase>
111111
struct ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC CompressedTupleImpl;
112112

113113
template <typename... Ts, size_t... I, bool ShouldAnyUseBase>
114-
struct ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC CompressedTupleImpl<
115-
CompressedTuple<Ts...>, absl::index_sequence<I...>, ShouldAnyUseBase>
114+
struct ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC
115+
CompressedTupleImpl<CompressedTuple<Ts...>, absl::index_sequence<I...>,
116+
ShouldAnyUseBase>
116117
// We use the dummy identity function through std::integral_constant to
117118
// convince MSVC of accepting and expanding I in that context. Without it
118119
// you would get:
119120
// error C3548: 'I': parameter pack cannot be used in this context
120-
: uses_inheritance,
121-
Storage<Ts, std::integral_constant<size_t, I>::value>... {
121+
: Storage<Ts, std::integral_constant<size_t, I>::value,
122+
StorageTag<Ts...>>... {
122123
constexpr CompressedTupleImpl() = default;
123124
template <typename... Vs>
124125
explicit constexpr CompressedTupleImpl(absl::in_place_t, Vs&&... args)
125-
: Storage<Ts, I>(absl::in_place, std::forward<Vs>(args))... {}
126+
: Storage<Ts, I, StorageTag<Ts...>>(absl::in_place,
127+
std::forward<Vs>(args))... {}
126128
friend CompressedTuple<Ts...>;
127129
};
128130

129131
template <typename... Ts, size_t... I>
130-
struct ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC CompressedTupleImpl<
131-
CompressedTuple<Ts...>, absl::index_sequence<I...>, false>
132+
struct ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC
133+
CompressedTupleImpl<CompressedTuple<Ts...>, absl::index_sequence<I...>,
134+
false>
132135
// We use the dummy identity function as above...
133-
: Storage<Ts, std::integral_constant<size_t, I>::value, false>... {
136+
: Storage<Ts, std::integral_constant<size_t, I>::value, StorageTag<Ts...>,
137+
false>... {
134138
constexpr CompressedTupleImpl() = default;
135139
template <typename... Vs>
136140
explicit constexpr CompressedTupleImpl(absl::in_place_t, Vs&&... args)
137-
: Storage<Ts, I, false>(absl::in_place, std::forward<Vs>(args))... {}
141+
: Storage<Ts, I, StorageTag<Ts...>, false>(absl::in_place,
142+
std::forward<Vs>(args))... {}
138143
friend CompressedTuple<Ts...>;
139144
};
140145

@@ -183,9 +188,7 @@ struct TupleItemsMoveConstructible
183188
// Helper class to perform the Empty Base Class Optimization.
184189
// Ts can contain classes and non-classes, empty or not. For the ones that
185190
// are empty classes, we perform the CompressedTuple. If all types in Ts are
186-
// empty classes, then CompressedTuple<Ts...> is itself an empty class. (This
187-
// does not apply when one or more of those empty classes is itself an empty
188-
// CompressedTuple.)
191+
// empty classes, then CompressedTuple<Ts...> is itself an empty class.
189192
//
190193
// To access the members, use member .get<N>() function.
191194
//
@@ -208,7 +211,8 @@ class ABSL_INTERNAL_COMPRESSED_TUPLE_DECLSPEC CompressedTuple
208211
using ElemT = internal_compressed_tuple::ElemT<CompressedTuple, I>;
209212

210213
template <int I>
211-
using StorageT = internal_compressed_tuple::Storage<ElemT<I>, I>;
214+
using StorageT = internal_compressed_tuple::Storage<
215+
ElemT<I>, I, internal_compressed_tuple::StorageTag<Ts...>>;
212216

213217
public:
214218
// There seems to be a bug in MSVC dealing in which using '=default' here will

absl/container/internal/compressed_tuple_test.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,14 @@ TEST(CompressedTupleTest, PointerToEmpty) {
107107
}
108108
}
109109

110+
TEST(CompressedTupleTest, NestedCompressedTuplePreservesEmptiness) {
111+
using TupleType = CompressedTuple<Empty<0>, CompressedTuple<Empty<0>>>;
112+
TupleType x;
113+
EXPECT_EQ(x.get<0>().value(), CallType::kMutableRef);
114+
EXPECT_EQ(x.get<1>().get<0>().value(), CallType::kMutableRef);
115+
EXPECT_TRUE(std::is_empty_v<TupleType>);
116+
}
117+
110118
TEST(CompressedTupleTest, OneMoveOnRValueConstructionTemp) {
111119
InstanceTracker tracker;
112120
CompressedTuple<CopyableMovableInstance> x1(CopyableMovableInstance(1));

0 commit comments

Comments
 (0)