Commit 1a8553b8 authored by Nick Terrell's avatar Nick Terrell Committed by Facebook Github Bot

Fix Expected stack overflow

Summary:
Fix https://github.com/facebook/folly/issues/1111 by calling
`assign()` directly. All `ExpectedStorage::assign()` implementations
eventually call `doEmplaceAssign()` which uses SFINAE to assign,
and if that fails emplace. Therefore, they only need `std::is_constructible`.

Reviewed By: yfeldblum

Differential Revision: D15756301

fbshipit-source-id: e7759128c13344ed6e3d58d6ea143fd68af8121b
parent 24606aea
......@@ -901,7 +901,7 @@ class Expected final : expected_detail::ExpectedStorage<Value, Error> {
std::is_constructible<Value, V&&>::value &&
std::is_constructible<Error, E&&>::value)>
Expected(Expected<V, E> that) : Base{expected_detail::EmptyTag{}} {
*this = std::move(that);
this->assign(std::move(that));
}
FOLLY_REQUIRES(std::is_copy_constructible<Value>::value)
......
......@@ -794,4 +794,92 @@ TEST(Expected, ThenOrThrow) {
Unexpected<E>::BadExpectedAccess);
}
}
namespace {
struct Source {};
struct SmallPODConstructTo {
SmallPODConstructTo() = default;
explicit SmallPODConstructTo(Source) {}
};
struct LargePODConstructTo {
explicit LargePODConstructTo(Source) {}
int64_t array[10];
};
struct NonPODConstructTo {
explicit NonPODConstructTo(Source) {}
NonPODConstructTo(NonPODConstructTo const&) {}
NonPODConstructTo& operator=(NonPODConstructTo const&) { return *this; }
};
struct ConvertTo {
explicit ConvertTo(Source) {}
ConvertTo& operator=(Source) { return *this; }
};
static_assert(
expected_detail::getStorageType<int, SmallPODConstructTo>() ==
expected_detail::StorageType::ePODStruct,
"SmallPODConstructTo is ePODStruct");
static_assert(
expected_detail::getStorageType<int, LargePODConstructTo>() ==
expected_detail::StorageType::ePODUnion,
"LargePODConstructTo is ePODUnion");
static_assert(
expected_detail::getStorageType<int, NonPODConstructTo>() ==
expected_detail::StorageType::eUnion,
"NonPODConstructTo is eUnion");
template <typename Target>
constexpr bool constructibleNotConvertible() {
return std::is_constructible<Target, Source>() &&
!expected_detail::IsConvertible<Source, Target>();
}
static_assert(constructibleNotConvertible<SmallPODConstructTo>(), "");
static_assert(constructibleNotConvertible<LargePODConstructTo>(), "");
static_assert(constructibleNotConvertible<NonPODConstructTo>(), "");
static_assert(expected_detail::IsConvertible<Source, ConvertTo>(),
"convertible");
} // namespace
TEST(Expected, GitHubIssue1111) {
// See https://github.com/facebook/folly/issues/1111
Expected<int, SmallPODConstructTo> a = folly::makeExpected<Source>(5);
EXPECT_EQ(a.value(), 5);
}
TEST(Expected, ConstructorConstructibleNotConvertible) {
const Expected<int, Source> v = makeExpected<Source>(5);
const Expected<int, Source> e = makeUnexpected(Source());
// Test construction and assignment for each ExpectedStorage backend
{
folly::Expected<int, SmallPODConstructTo> cv(v);
folly::Expected<int, SmallPODConstructTo> ce(e);
cv = v;
ce = e;
}
{
folly::Expected<int, LargePODConstructTo> cv(v);
folly::Expected<int, LargePODConstructTo> ce(e);
cv = v;
ce = e;
}
{
folly::Expected<int, NonPODConstructTo> cv(v);
folly::Expected<int, NonPODConstructTo> ce(e);
cv = v;
ce = e;
}
// Test convertible construction and assignment
{
folly::Expected<int, ConvertTo> cv(v);
folly::Expected<int, ConvertTo> ce(e);
cv = v;
ce = e;
}
}
} // namespace folly
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