rtc::Optional<T>: Don't secretly contain a default-constructed T when empty

Instead, use a neat trick with union to ensure that we have a T only
when we're supposed to (and just a bunch of unused memory otherwise).
This is how std::optional behaves, so it makes sense for us to do the
same (and it's convenient, too, since we don't have to pay for the
default-constructed T, and we support types that don't have default
constructors).

Doing this became possible recently when we dropped support for MSVC
2013, which didn't support unions containing non-trivial types.

Review-Url: https://codereview.webrtc.org/1896833004
Cr-Commit-Position: refs/heads/master@{#12664}
This commit is contained in:
kwiberg 2016-05-09 06:06:05 -07:00 committed by Commit bot
parent e30c272051
commit d040480f69
2 changed files with 134 additions and 82 deletions

View File

@ -19,11 +19,7 @@
namespace rtc {
// Simple std::experimental::optional-wannabe. It either contains a T or not.
// In order to keep the implementation simple and portable, this implementation
// actually contains a (default-constructed) T even when it supposedly doesn't
// contain a value; use e.g. std::unique_ptr<T> instead if that's too
// expensive.
// Simple std::optional-wannabe. It either contains a T or not.
//
// A moved-from Optional<T> may only be destroyed, and assigned to if T allows
// being assigned to after having been moved from. Specifically, you may not
@ -66,21 +62,90 @@ class Optional final {
Optional() : has_value_(false) {}
// Construct an Optional that contains a value.
explicit Optional(const T& val) : value_(val), has_value_(true) {}
explicit Optional(T&& val) : value_(std::move(val)), has_value_(true) {}
explicit Optional(const T& value) : has_value_(true) {
new (&value_) T(value);
}
explicit Optional(T&& value) : has_value_(true) {
new (&value_) T(std::move(value));
}
// Copy and move constructors.
Optional(const Optional&) = default;
Optional(Optional&&) = default;
// Copy constructor: copies the value from m if it has one.
Optional(const Optional& m) : has_value_(m.has_value_) {
if (has_value_)
new (&value_) T(m.value_);
}
// Assignment.
Optional& operator=(const Optional&) = default;
Optional& operator=(Optional&&) = default;
// Move constructor: if m has a value, moves the value from m, leaving m
// still in a state where it has a value, but a moved-from one (the
// properties of which depends on T; the only general guarantee is that we
// can destroy m).
Optional(Optional&& m) : has_value_(m.has_value_) {
if (has_value_)
new (&value_) T(std::move(m.value_));
}
~Optional() {
if (has_value_)
value_.~T();
}
// Copy assignment. Uses T's copy assignment if both sides have a value, T's
// copy constructor if only the right-hand side has a value.
Optional& operator=(const Optional& m) {
if (m.has_value_) {
if (has_value_) {
value_ = m.value_; // T's copy assignment.
} else {
new (&value_) T(m.value_); // T's copy constructor.
has_value_ = true;
}
} else if (has_value_) {
value_.~T();
has_value_ = false;
}
return *this;
}
// Move assignment. Uses T's move assignment if both sides have a value, T's
// move constructor if only the right-hand side has a value. The state of m
// after it's been moved from is as for the move constructor.
Optional& operator=(Optional&& m) {
if (m.has_value_) {
if (has_value_) {
value_ = std::move(m.value_); // T's move assignment.
} else {
new (&value_) T(std::move(m.value_)); // T's move constructor.
has_value_ = true;
}
} else if (has_value_) {
value_.~T();
has_value_ = false;
}
return *this;
}
// Swap the values if both m1 and m2 have values; move the value if only one
// of them has one.
friend void swap(Optional& m1, Optional& m2) {
using std::swap;
swap(m1.value_, m2.value_);
swap(m1.has_value_, m2.has_value_);
if (m1.has_value_) {
if (m2.has_value_) {
// Both have values: swap.
using std::swap;
swap(m1.value_, m2.value_);
} else {
// Only m1 has a value: move it to m2.
new (&m2.value_) T(std::move(m1.value_));
m1.value_.~T(); // Destroy the moved-from value.
m1.has_value_ = false;
m2.has_value_ = true;
}
} else if (m2.has_value_) {
// Only m2 has a value: move it to m1.
new (&m1.value_) T(std::move(m2.value_));
m2.value_.~T(); // Destroy the moved-from value.
m1.has_value_ = true;
m2.has_value_ = false;
}
}
// Conversion to bool to test if we have a value.
@ -122,10 +187,15 @@ class Optional final {
}
private:
// Invariant: Unless *this has been moved from, value_ is default-initialized
// (or copied or moved from a default-initialized T) if !has_value_.
T value_;
bool has_value_;
bool has_value_; // True iff value_ contains a live value.
union {
// By placing value_ in a union, we get to manage its construction and
// destruction manually: the Optional constructors won't automatically
// construct it, and the Optional destructor won't automatically destroy
// it. Basically, this just allocates a properly sized and aligned block of
// memory in which we can manually put a T with placement new.
T value_;
};
};
} // namespace rtc

View File

@ -125,7 +125,7 @@ TEST(OptionalTest, TestConstructDefault) {
Optional<Logger> x;
EXPECT_FALSE(x);
}
EXPECT_EQ(V("0:0. default constructor", "0:0. destructor"), *log);
EXPECT_EQ(V(), *log);
}
TEST(OptionalTest, TestConstructCopyEmpty) {
@ -136,9 +136,7 @@ TEST(OptionalTest, TestConstructCopyEmpty) {
auto y = x;
EXPECT_FALSE(y);
}
EXPECT_EQ(V("0:0. default constructor", "1:0. copy constructor (from 0:0)",
"1:0. destructor", "0:0. destructor"),
*log);
EXPECT_EQ(V(), *log);
}
TEST(OptionalTest, TestConstructCopyFull) {
@ -166,9 +164,7 @@ TEST(OptionalTest, TestConstructMoveEmpty) {
auto y = std::move(x);
EXPECT_FALSE(y);
}
EXPECT_EQ(V("0:0. default constructor", "1:0. move constructor (from 0:0)",
"1:0. destructor", "0:0. destructor"),
*log);
EXPECT_EQ(V(), *log);
}
TEST(OptionalTest, TestConstructMoveFull) {
@ -195,10 +191,7 @@ TEST(OptionalTest, TestCopyAssignToEmptyFromEmpty) {
Optional<Logger> x, y;
x = y;
}
EXPECT_EQ(
V("0:0. default constructor", "1:1. default constructor",
"0:1. operator= copy (from 1:1)", "1:1. destructor", "0:1. destructor"),
*log);
EXPECT_EQ(V(), *log);
}
TEST(OptionalTest, TestCopyAssignToFullFromEmpty) {
@ -212,9 +205,7 @@ TEST(OptionalTest, TestCopyAssignToFullFromEmpty) {
}
EXPECT_EQ(
V("0:17. explicit constructor", "1:17. move constructor (from 0:17)",
"0:17. destructor", "2:2. default constructor", "---",
"1:2. operator= copy (from 2:2)", "---", "2:2. destructor",
"1:2. destructor"),
"0:17. destructor", "---", "1:17. destructor", "---"),
*log);
}
@ -227,11 +218,11 @@ TEST(OptionalTest, TestCopyAssignToEmptyFromFull) {
x = y;
log->push_back("---");
}
EXPECT_EQ(V("0:0. default constructor", "1:17. explicit constructor",
"2:17. move constructor (from 1:17)", "1:17. destructor", "---",
"0:17. operator= copy (from 2:17)", "---", "2:17. destructor",
"0:17. destructor"),
*log);
EXPECT_EQ(
V("0:17. explicit constructor", "1:17. move constructor (from 0:17)",
"0:17. destructor", "---", "2:17. copy constructor (from 1:17)", "---",
"1:17. destructor", "2:17. destructor"),
*log);
}
TEST(OptionalTest, TestCopyAssignToFullFromFull) {
@ -261,10 +252,10 @@ TEST(OptionalTest, TestCopyAssignToEmptyFromT) {
x = Optional<Logger>(y);
log->push_back("---");
}
EXPECT_EQ(V("0:0. default constructor", "1:17. explicit constructor", "---",
"2:17. copy constructor (from 1:17)",
"0:17. operator= move (from 2:17)", "2:17. destructor", "---",
"1:17. destructor", "0:17. destructor"),
EXPECT_EQ(V("0:17. explicit constructor", "---",
"1:17. copy constructor (from 0:17)",
"2:17. move constructor (from 1:17)", "1:17. destructor", "---",
"0:17. destructor", "2:17. destructor"),
*log);
}
@ -292,10 +283,7 @@ TEST(OptionalTest, TestMoveAssignToEmptyFromEmpty) {
Optional<Logger> x, y;
x = std::move(y);
}
EXPECT_EQ(
V("0:0. default constructor", "1:1. default constructor",
"0:1. operator= move (from 1:1)", "1:1. destructor", "0:1. destructor"),
*log);
EXPECT_EQ(V(), *log);
}
TEST(OptionalTest, TestMoveAssignToFullFromEmpty) {
@ -309,9 +297,7 @@ TEST(OptionalTest, TestMoveAssignToFullFromEmpty) {
}
EXPECT_EQ(
V("0:17. explicit constructor", "1:17. move constructor (from 0:17)",
"0:17. destructor", "2:2. default constructor", "---",
"1:2. operator= move (from 2:2)", "---", "2:2. destructor",
"1:2. destructor"),
"0:17. destructor", "---", "1:17. destructor", "---"),
*log);
}
@ -324,11 +310,11 @@ TEST(OptionalTest, TestMoveAssignToEmptyFromFull) {
x = std::move(y);
log->push_back("---");
}
EXPECT_EQ(V("0:0. default constructor", "1:17. explicit constructor",
"2:17. move constructor (from 1:17)", "1:17. destructor", "---",
"0:17. operator= move (from 2:17)", "---", "2:17. destructor",
"0:17. destructor"),
*log);
EXPECT_EQ(
V("0:17. explicit constructor", "1:17. move constructor (from 0:17)",
"0:17. destructor", "---", "2:17. move constructor (from 1:17)", "---",
"1:17. destructor", "2:17. destructor"),
*log);
}
TEST(OptionalTest, TestMoveAssignToFullFromFull) {
@ -358,10 +344,10 @@ TEST(OptionalTest, TestMoveAssignToEmptyFromT) {
x = Optional<Logger>(std::move(y));
log->push_back("---");
}
EXPECT_EQ(V("0:0. default constructor", "1:17. explicit constructor", "---",
"2:17. move constructor (from 1:17)",
"0:17. operator= move (from 2:17)", "2:17. destructor", "---",
"1:17. destructor", "0:17. destructor"),
EXPECT_EQ(V("0:17. explicit constructor", "---",
"1:17. move constructor (from 0:17)",
"2:17. move constructor (from 1:17)", "1:17. destructor", "---",
"0:17. destructor", "2:17. destructor"),
*log);
}
@ -426,14 +412,13 @@ TEST(OptionalTest, TestDereferenceWithDefault) {
}
EXPECT_EQ(
V("0:17. explicit constructor", "1:42. explicit constructor",
"2:17. copy constructor (from 0:17)", "3:3. default constructor", "-1-",
"4:42. explicit constructor", "operator== 0:17, 2:17",
"4:42. destructor", "-2-", "5:42. explicit constructor",
"operator== 1:42, 5:42", "5:42. destructor", "-3-",
"6:17. explicit constructor", "7:17. move constructor (from 6:17)",
"operator== 0:17, 7:17", "7:17. destructor", "6:17. destructor", "-4-",
"8:8. default constructor", "operator== 1:42, 1:42", "8:8. destructor",
"-5-", "3:3. destructor", "2:17. destructor", "1:42. destructor",
"2:17. copy constructor (from 0:17)", "-1-",
"3:42. explicit constructor", "operator== 0:17, 2:17",
"3:42. destructor", "-2-", "4:42. explicit constructor",
"operator== 1:42, 4:42", "4:42. destructor", "-3-",
"5:17. explicit constructor", "6:17. move constructor (from 5:17)",
"operator== 0:17, 6:17", "6:17. destructor", "5:17. destructor", "-4-",
"operator== 1:42, 1:42", "-5-", "2:17. destructor", "1:42. destructor",
"0:17. destructor"),
*log);
}
@ -452,16 +437,15 @@ TEST(OptionalTest, TestEquality) {
EXPECT_EQ(me1, me2);
log->push_back("---");
}
EXPECT_EQ(V("0:17. explicit constructor", "1:42. explicit constructor",
"2:17. copy constructor (from 0:17)",
"3:17. copy constructor (from 0:17)",
"4:42. copy constructor (from 1:42)", "5:5. default constructor",
"6:6. default constructor", "---", "operator== 2:17, 2:17",
"operator== 2:17, 3:17", "operator!= 2:17, 4:42", "---",
"6:6. destructor", "5:5. destructor", "4:42. destructor",
"3:17. destructor", "2:17. destructor", "1:42. destructor",
"0:17. destructor"),
*log);
EXPECT_EQ(
V("0:17. explicit constructor", "1:42. explicit constructor",
"2:17. copy constructor (from 0:17)",
"3:17. copy constructor (from 0:17)",
"4:42. copy constructor (from 1:42)", "---", "operator== 2:17, 2:17",
"operator== 2:17, 3:17", "operator!= 2:17, 4:42", "---",
"4:42. destructor", "3:17. destructor", "2:17. destructor",
"1:42. destructor", "0:17. destructor"),
*log);
}
TEST(OptionalTest, TestSwap) {
@ -478,11 +462,9 @@ TEST(OptionalTest, TestSwap) {
EXPECT_EQ(V("0:17. explicit constructor", "1:42. explicit constructor",
"2:17. copy constructor (from 0:17)",
"3:42. copy constructor (from 1:42)",
"4:17. copy constructor (from 0:17)", "5:5. default constructor",
"6:6. default constructor", "7:7. default constructor", "---",
"swap 2:42, 3:17", "swap 4:5, 5:17", "swap 6:7, 7:6", "---",
"7:6. destructor", "6:7. destructor", "5:17. destructor",
"4:5. destructor", "3:17. destructor", "2:42. destructor",
"4:17. copy constructor (from 0:17)", "---", "swap 2:42, 3:17",
"5:17. move constructor (from 4:17)", "4:17. destructor", "---",
"5:17. destructor", "3:17. destructor", "2:42. destructor",
"1:42. destructor", "0:17. destructor"),
*log);
}