[llvm] 1d43966 - [llvm][ADT] Allow returning `std::nullopt` in TypeSwitch
Markus Böck via llvm-commits
llvm-commits at lists.llvm.org
Sat Dec 17 15:02:05 PST 2022
Author: Markus Böck
Date: 2022-12-18T00:02:03+01:00
New Revision: 1d43966bc33a55cad1db7758bf4d82526d125db7
URL: https://github.com/llvm/llvm-project/commit/1d43966bc33a55cad1db7758bf4d82526d125db7
DIFF: https://github.com/llvm/llvm-project/commit/1d43966bc33a55cad1db7758bf4d82526d125db7.diff
LOG: [llvm][ADT] Allow returning `std::nullopt` in TypeSwitch
Returning `std::nullopt` from the case of a `TypeSwitch` yields broken results, by either falling through to another case, or falling of the switch entirely and hitting an assertion. This is simply due to the use of `operator=` of what is now `std::optional`, which has an overload specifically for `std::nullopt`, causing the internal optional, used for the `TypeSwitch` result to be reset, instead of a value being constructed from the `std::nullopt`.
The fix is to simply use the `emplace` method of `std::optional`, causing a value to always be constructed from the value returned by the case function.
Differential Revision: https://reviews.llvm.org/D140265
Added:
Modified:
llvm/include/llvm/ADT/TypeSwitch.h
llvm/unittests/ADT/TypeSwitchTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ADT/TypeSwitch.h b/llvm/include/llvm/ADT/TypeSwitch.h
index f9323a50891c..10a2d48e918d 100644
--- a/llvm/include/llvm/ADT/TypeSwitch.h
+++ b/llvm/include/llvm/ADT/TypeSwitch.h
@@ -119,7 +119,7 @@ class TypeSwitch : public detail::TypeSwitchBase<TypeSwitch<T, ResultT>, T> {
// Check to see if CaseT applies to 'value'.
if (auto caseValue = BaseT::template castValue<CaseT>(this->value))
- result = caseFn(caseValue);
+ result.emplace(caseFn(caseValue));
return *this;
}
diff --git a/llvm/unittests/ADT/TypeSwitchTest.cpp b/llvm/unittests/ADT/TypeSwitchTest.cpp
index 442ac1910a05..c54b7987edf7 100644
--- a/llvm/unittests/ADT/TypeSwitchTest.cpp
+++ b/llvm/unittests/ADT/TypeSwitchTest.cpp
@@ -86,3 +86,31 @@ TEST(TypeSwitchTest, CasesVoid) {
EXPECT_EQ(0, translate(DerivedD()));
EXPECT_EQ(-1, translate(DerivedE()));
}
+
+TEST(TypeSwitchTest, CaseOptional) {
+ auto translate = [](auto value) {
+ return TypeSwitch<Base *, std::optional<int>>(&value)
+ .Case([](DerivedA *) { return 0; })
+ .Case([](DerivedC *) { return std::nullopt; })
+ .Default([](Base *) { return -1; });
+ };
+ EXPECT_EQ(0, translate(DerivedA()));
+ EXPECT_EQ(std::nullopt, translate(DerivedC()));
+ EXPECT_EQ(-1, translate(DerivedD()));
+ EXPECT_EQ(std::nullopt,
+ (TypeSwitch<Base *, std::optional<int>>(nullptr).Default(
+ [](Base *) { return std::nullopt; })));
+}
+
+TEST(TypeSwitchTest, CasesOptional) {
+ auto translate = [](auto value) {
+ return TypeSwitch<Base *, std::optional<int>>(&value)
+ .Case<DerivedB, DerivedC>([](auto *) { return std::nullopt; })
+ .Case([](DerivedA *) { return 0; })
+ .Default([](Base *) { return -1; });
+ };
+ EXPECT_EQ(0, translate(DerivedA()));
+ EXPECT_EQ(std::nullopt, translate(DerivedB()));
+ EXPECT_EQ(std::nullopt, translate(DerivedC()));
+ EXPECT_EQ(-1, translate(DerivedD()));
+}
More information about the llvm-commits
mailing list