Skip to content

Commit fadbc88

Browse files
Yi Zhangfacebook-github-bot
authored andcommitted
Fix bug in folly::Optional::toStdOptional
Summary: See https://fb.workplace.com/groups/145730152809376/permalink/1685600578822318/ for context. If you call toStdOptional on folly::Optional<bool>, the following cast will not work as expected: ``` folly::Optional<bool> optional = false; // This will convert optional to bool and then construct std::optional<value>! auto value = static_cast<std::optional<Value>>(optional); ``` So we just need to not rely on the cast and implement it in the more verbose way. Differential Revision: D79928399 fbshipit-source-id: ea41ff6bf9c7724704b7d7056c0fd812e1faf0b6
1 parent 5de4b71 commit fadbc88

File tree

2 files changed

+34
-2
lines changed

2 files changed

+34
-2
lines changed

folly/Optional.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,19 @@ class Optional {
220220
}
221221

222222
std::optional<Value> toStdOptional() && noexcept {
223-
return static_cast<std::optional<Value>>(std::move(*this));
223+
if (has_value()) {
224+
auto opt = std::optional<Value>(std::move(value()));
225+
reset();
226+
return opt;
227+
}
228+
return std::nullopt;
224229
}
225230

226231
std::optional<Value> toStdOptional() const& noexcept {
227-
return static_cast<std::optional<Value>>(*this);
232+
if (has_value()) {
233+
return std::optional<Value>(value());
234+
}
235+
return std::nullopt;
228236
}
229237

230238
/// Set the Optional

folly/test/OptionalTest.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,30 @@ TEST(Optional, StdOptionalConversions) {
844844
EXPECT_EQ(s, f.toStdOptional());
845845
EXPECT_TRUE(f);
846846

847+
folly::Optional<int> f_empty;
848+
std::optional<int> s_empty = static_cast<std::optional<int>>(f_empty);
849+
EXPECT_FALSE(s_empty.has_value());
850+
851+
folly::Optional<bool> bf = false;
852+
std::optional<bool> bs = bf.toStdOptional();
853+
EXPECT_EQ(bs.has_value(), true);
854+
EXPECT_EQ(bs.value(), false);
855+
856+
folly::Optional<bool> bf_empty;
857+
std::optional<bool> bs_empty = bf_empty.toStdOptional();
858+
EXPECT_FALSE(bs_empty.has_value());
859+
860+
folly::Optional<bool> mbf = false;
861+
auto mbs = std::move(mbf).toStdOptional();
862+
EXPECT_TRUE(mbs.has_value());
863+
EXPECT_FALSE(mbs.value());
864+
EXPECT_FALSE(mbf.has_value());
865+
866+
folly::Optional<bool> mbf_empty;
867+
auto mbs_empty = std::move(mbf_empty).toStdOptional();
868+
EXPECT_FALSE(mbs_empty.has_value());
869+
EXPECT_FALSE(mbf_empty.has_value());
870+
847871
f = static_cast<folly::Optional<int>>(s);
848872
EXPECT_EQ(*f, 42);
849873
EXPECT_TRUE(s);

0 commit comments

Comments
 (0)