Commit fd20c97c authored by Lovro Puzar's avatar Lovro Puzar Committed by Jordan DeLong

Add explicit assignment operator definitions to Optional

Summary:
See new test.  Under GCC 4.6 (which is what the folly tests build with) the old code works fine but under 4.7 building fails with:

folly/test/OptionalTest.cpp: In member function ‘virtual void Optional_AssignmentContained_Test::TestBody()’:
folly/test/OptionalTest.cpp:122:14: error: use of deleted function ‘ContainsOptional& ContainsOptional::operator=(const ContainsOptional&)’
folly/test/OptionalTest.cpp:106:21: note: ‘ContainsOptional& ContainsOptional::operator=(const ContainsOptional&)’ is implicitly deleted because the default definition would be ill-formed:
folly/test/OptionalTest.cpp:106:21: error: use of deleted function ‘folly::Optional<int>& folly::Optional<int>::operator=(const folly::Optional<int>&)’
In file included from folly/test/OptionalTest.cpp:17:0:
./folly/Optional.h:84:7: note: ‘folly::Optional<int>& folly::Optional<int>::operator=(const folly::Optional<int>&)’ is implicitly declared as deleted because ‘folly::Optional<int>’ declares a move constructor or move assignment operator
folly/test/OptionalTest.cpp:129:30: error: use of deleted function ‘ContainsOptional& ContainsOptional::operator=(ContainsOptional&&)’
folly/test/OptionalTest.cpp:108:21: note: ‘ContainsOptional& ContainsOptional::operator=(ContainsOptional&&)’ is implicitly deleted because the default definition would be ill-formed:
folly/test/OptionalTest.cpp:108:21: error: non-static data member ‘ContainsOptional::opt_’ does not have a move assignment operator or trivial copy assignment operator
folly/test/OptionalTest.cpp:137:14: error: use of deleted function ‘ContainsOptional& ContainsOptional::operator=(const ContainsOptional&)’

Test Plan:
(1) fbconfig folly/test && fbmake dbg && _build/dbg/folly/test/optional_test
(2) Remove folly/PLATFORM to build with gcc 4.7, then repeat (1).  Without the code fix, the new test fails to build.  With the fix, the test builds and runs fine.

Reviewed By: tjackson@fb.com

FB internal diff: D732402
parent d006f324
......@@ -169,6 +169,16 @@ class Optional : boost::totally_ordered<Optional<Value>,
return *this;
}
Optional& operator=(Optional &&other) {
assign(std::move(other));
return *this;
}
Optional& operator=(const Optional &other) {
assign(other);
return *this;
}
bool operator<(const Optional& other) const {
if (hasValue() != other.hasValue()) {
return hasValue() < other.hasValue();
......
......@@ -309,3 +309,47 @@ TEST(Optional, MakeOptional) {
ASSERT_TRUE(optIntPtr.hasValue());
EXPECT_EQ(**optIntPtr, 3);
}
class ContainsOptional {
public:
ContainsOptional() { }
explicit ContainsOptional(int x) : opt_(x) { }
bool hasValue() const { return opt_.hasValue(); }
int value() const { return opt_.value(); }
ContainsOptional(const ContainsOptional &other) = default;
ContainsOptional& operator=(const ContainsOptional &other) = default;
ContainsOptional(ContainsOptional &&other) = default;
ContainsOptional& operator=(ContainsOptional &&other) = default;
private:
Optional<int> opt_;
};
/**
* Test that a class containing an Optional can be copy and move assigned.
* This was broken under gcc 4.7 until assignment operators were explicitly
* defined.
*/
TEST(Optional, AssignmentContained) {
{
ContainsOptional source(5), target;
target = source;
EXPECT_TRUE(target.hasValue());
EXPECT_EQ(5, target.value());
}
{
ContainsOptional source(5), target;
target = std::move(source);
EXPECT_TRUE(target.hasValue());
EXPECT_EQ(5, target.value());
EXPECT_FALSE(source.hasValue());
}
{
ContainsOptional opt_uninit, target(10);
target = opt_uninit;
EXPECT_FALSE(target.hasValue());
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment