Skip to content

Commit a2f6dd4

Browse files
committed
[NFC] Appropriately delete and noexcept TaggedUnion's special members
1 parent 63db545 commit a2f6dd4

File tree

2 files changed

+116
-12
lines changed

2 files changed

+116
-12
lines changed

include/swift/Basic/ExternalUnion.h

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,17 @@ struct ExternalUnionMembers {
7070
// (private to the implementation)
7171
using Info = ExternalUnionImpl::MembersHelper<Members...>;
7272

73+
enum : bool {
74+
is_copy_constructible = Info::is_copy_constructible,
75+
is_nothrow_copy_constructible = Info::is_nothrow_copy_constructible,
76+
is_move_constructible = Info::is_move_constructible,
77+
is_nothrow_move_constructible = Info::is_nothrow_move_constructible,
78+
is_copy_assignable = Info::is_copy_assignable,
79+
is_nothrow_copy_assignable = Info::is_nothrow_copy_assignable,
80+
is_move_assignable = Info::is_move_assignable,
81+
is_nothrow_move_assignable = Info::is_nothrow_move_assignable,
82+
};
83+
7384
/// The type of indices into the union member type list.
7485
enum Index : unsigned {};
7586

@@ -429,7 +440,15 @@ namespace ExternalUnionImpl {
429440
template <>
430441
struct MembersHelper<> {
431442
enum : bool {
432-
is_trivially_copyable = true
443+
is_trivially_copyable = true,
444+
is_copy_constructible = true,
445+
is_nothrow_copy_constructible = true,
446+
is_move_constructible = true,
447+
is_nothrow_move_constructible = true,
448+
is_copy_assignable = true,
449+
is_nothrow_copy_assignable = true,
450+
is_move_assignable = true,
451+
is_nothrow_move_assignable = true,
433452
};
434453

435454
enum : size_t {
@@ -476,7 +495,32 @@ struct MembersHelper<H, T...> {
476495
public:
477496
enum : bool {
478497
is_trivially_copyable =
479-
Member::is_trivially_copyable && Others::is_trivially_copyable
498+
Member::is_trivially_copyable &&
499+
Others::is_trivially_copyable,
500+
is_copy_constructible =
501+
Member::is_copy_constructible &&
502+
Others::is_copy_constructible,
503+
is_nothrow_copy_constructible =
504+
Member::is_nothrow_copy_constructible &&
505+
Others::is_nothrow_copy_constructible,
506+
is_move_constructible =
507+
Member::is_move_constructible &&
508+
Others::is_move_constructible,
509+
is_nothrow_move_constructible =
510+
Member::is_nothrow_move_constructible &&
511+
Others::is_nothrow_move_constructible,
512+
is_copy_assignable =
513+
Member::is_copy_assignable &&
514+
Others::is_copy_assignable,
515+
is_nothrow_copy_assignable =
516+
Member::is_nothrow_copy_assignable &&
517+
Others::is_nothrow_copy_assignable,
518+
is_move_assignable =
519+
Member::is_move_assignable &&
520+
Others::is_move_assignable,
521+
is_nothrow_move_assignable =
522+
Member::is_nothrow_move_assignable &&
523+
Others::is_nothrow_move_assignable,
480524
};
481525

482526
enum : size_t {
@@ -536,7 +580,19 @@ struct MembersHelper<H, T...> {
536580
template <class T>
537581
struct UnionMemberInfo {
538582
enum : bool {
539-
is_trivially_copyable = IsTriviallyCopyable<T>::value
583+
is_trivially_copyable = IsTriviallyCopyable<T>::value,
584+
is_copy_constructible = std::is_copy_constructible<T>::value,
585+
is_nothrow_copy_constructible =
586+
std::is_nothrow_copy_constructible<T>::value,
587+
is_move_constructible = std::is_move_constructible<T>::value,
588+
is_nothrow_move_constructible =
589+
std::is_nothrow_move_constructible<T>::value,
590+
is_copy_assignable = std::is_copy_assignable<T>::value,
591+
is_nothrow_copy_assignable =
592+
std::is_nothrow_copy_assignable<T>::value,
593+
is_move_assignable = std::is_move_assignable<T>::value,
594+
is_nothrow_move_assignable =
595+
std::is_nothrow_move_assignable<T>::value,
540596
};
541597

542598
enum : size_t {
@@ -575,7 +631,15 @@ struct UnionMemberInfo {
575631
template <>
576632
struct UnionMemberInfo<void> {
577633
enum : bool {
578-
is_trivially_copyable = true
634+
is_trivially_copyable = true,
635+
is_copy_constructible = true,
636+
is_nothrow_copy_constructible = true,
637+
is_move_constructible = true,
638+
is_nothrow_move_constructible = true,
639+
is_copy_assignable = true,
640+
is_nothrow_copy_assignable = true,
641+
is_move_assignable = true,
642+
is_nothrow_move_assignable = true,
579643
};
580644

581645
enum : size_t {

include/swift/Basic/TaggedUnion.h

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,26 +163,60 @@ class TaggedUnionBase<KindHelper, Members, /*NonTrivial*/ true, /*HasVoid*/ fals
163163
TaggedUnionImpl::Empty>::type = {})
164164
: super(std::forward<T>(value)) {}
165165

166-
TaggedUnionBase(const TaggedUnionBase &other) : super(other.TheKind) {
166+
// We want to either define or delete all the special members.
167+
// C++ does not provide a direct way to conditionally delete a
168+
// function. enable_if doesn't work, for several reasons: you can't
169+
// make an enable_if condition depend only on a property of the
170+
// enclosing class template (because all member function signatures
171+
// must successfully instantiate as part of instantiating the class
172+
// template; this is not covered by SFINAE), and you can't make the
173+
// special member itself a template (because then it's not a special
174+
// member anymore). static_assert within the member also doesn't work.
175+
// But we *can* make template substitution decide whether something
176+
// turns into a special member, then declare it both ways.
177+
template <bool condition>
178+
using self_if = typename std::conditional<condition,
179+
TaggedUnionBase,
180+
TaggedUnionImpl::Empty>::type;
181+
182+
TaggedUnionBase(const self_if<Members::is_copy_constructible> &other)
183+
noexcept(Members::is_nothrow_copy_constructible)
184+
: super(other.TheKind) {
167185
Storage.copyConstruct(other.TheKind, other.Storage);
168186
}
169187

170-
TaggedUnionBase(TaggedUnionBase &&other) : super(other.TheKind) {
188+
TaggedUnionBase(const self_if<!Members::is_copy_constructible> &other)
189+
= delete;
190+
191+
TaggedUnionBase(self_if<Members::is_move_constructible> &&other)
192+
noexcept(Members::is_nothrow_move_constructible)
193+
: super(other.TheKind) {
171194
Storage.moveConstruct(other.TheKind, std::move(other.Storage));
172195
}
173196

174-
TaggedUnionBase &operator=(const TaggedUnionBase &other) {
197+
TaggedUnionBase(self_if<!Members::is_move_constructible> &&other)
198+
= delete;
199+
200+
TaggedUnionBase &operator=(const self_if<Members::is_copy_assignable> &other)
201+
noexcept(Members::is_nothrow_copy_assignable) {
175202
Storage.copyAssign(TheKind, other.TheKind, other.Storage);
176203
TheKind = other.TheKind;
177204
return *this;
178205
}
179206

180-
TaggedUnionBase &operator=(TaggedUnionBase &&other) {
207+
TaggedUnionBase &operator=(const self_if<!Members::is_copy_assignable> &other)
208+
= delete;
209+
210+
TaggedUnionBase &operator=(self_if<Members::is_move_assignable> &&other)
211+
noexcept(Members::is_nothrow_move_assignable) {
181212
Storage.moveAssign(TheKind, other.TheKind, std::move(other.Storage));
182213
TheKind = other.TheKind;
183214
return *this;
184215
}
185216

217+
TaggedUnionBase &operator=(self_if<!Members::is_move_assignable> &&other)
218+
= delete;
219+
186220
~TaggedUnionBase() {
187221
Storage.destruct(TheKind);
188222
}
@@ -250,10 +284,16 @@ class TaggedUnionBase<KindHelper, Members, NonTrivial, /*HasVoid*/ true>
250284
/// empty state and provides a default constructor as well as `empty()`
251285
/// and `reset()` methods.
252286
template <class... Members>
253-
using TaggedUnion =
254-
TaggedUnionBase<ExternalUnionImpl::OptimalKindTypeHelper<sizeof...(Members)>,
255-
ExternalUnionMembers<Members...>>;
287+
class TaggedUnion :
288+
public TaggedUnionBase<ExternalUnionImpl::OptimalKindTypeHelper<sizeof...(Members)>,
289+
ExternalUnionMembers<Members...>> {
290+
using super =
291+
TaggedUnionBase<ExternalUnionImpl::OptimalKindTypeHelper<sizeof...(Members)>,
292+
ExternalUnionMembers<Members...>>;
293+
public:
294+
using super::super;
295+
};
256296

257-
}
297+
} // end namespace swift
258298

259299
#endif

0 commit comments

Comments
 (0)