From f7357f89a38a95bed97612f417100214a273b23f Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Thu, 5 May 2022 19:18:00 +0000 Subject: [PATCH] tint: Castable - support non-default-constructable return types If the Switch() has a default case, then allow support for return types that do not have a default constructor. Bug: tint:1504 Change-Id: I671ea78fe976138a786e2e0472e1e5f99afa0c5d Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/89022 Commit-Queue: Ben Clayton Kokoro: Kokoro Reviewed-by: Antonio Maiorano --- src/tint/castable.h | 85 +++++++++++++++++++++++++++------------ src/tint/castable_test.cc | 26 ++++++++++++ 2 files changed, 85 insertions(+), 26 deletions(-) diff --git a/src/tint/castable.h b/src/tint/castable.h index e0bb2bd9c0..048d1e5258 100644 --- a/src/tint/castable.h +++ b/src/tint/castable.h @@ -21,7 +21,9 @@ #include #include "src/tint/traits.h" +#include "src/tint/utils/bitcast.h" #include "src/tint/utils/crc32.h" +#include "src/tint/utils/defer.h" #if defined(__clang__) /// Temporarily disable certain warnings when using Castable API @@ -588,7 +590,7 @@ inline bool NonDefaultCases(T* object, if (type->Is(&TypeInfo::Of())) { auto* ptr = static_cast(object); if constexpr (kHasReturnType) { - *result = static_cast(std::get<0>(cases)(ptr)); + new (result) RETURN_TYPE(static_cast(std::get<0>(cases)(ptr))); } else { std::get<0>(cases)(ptr); } @@ -617,36 +619,61 @@ inline bool NonDefaultCases(T* object, template inline void SwitchCases(T* object, RETURN_TYPE* result, std::tuple&& cases) { using Cases = std::tuple; + static constexpr int kDefaultIndex = detail::IndexOfDefaultCase(); - static_assert(kDefaultIndex == -1 || kDefaultIndex == std::tuple_size_v - 1, - "Default case must be last in Switch()"); static constexpr bool kHasDefaultCase = kDefaultIndex >= 0; static constexpr bool kHasReturnType = !std::is_same_v; - if (object) { - auto* type = &object->TypeInfo(); - if constexpr (kHasDefaultCase) { - // Evaluate non-default cases. - if (!detail::NonDefaultCases(object, type, result, - traits::Slice<0, kDefaultIndex>(cases))) { - // Nothing matched. Evaluate default case. - if constexpr (kHasReturnType) { - *result = static_cast(std::get(cases)({})); - } else { - std::get(cases)({}); + // Static assertions + static constexpr bool kDefaultIsOK = + kDefaultIndex == -1 || kDefaultIndex == std::tuple_size_v - 1; + static constexpr bool kReturnIsOK = + kHasDefaultCase || !kHasReturnType || std::is_constructible_v; + static_assert(kDefaultIsOK, "Default case must be last in Switch()"); + static_assert(kReturnIsOK, + "Switch() requires either a Default case or a return type that is either void or " + "default-constructable"); + + // If the static asserts have fired, don't bother spewing more errors below + static constexpr bool kAllOK = kDefaultIsOK && kReturnIsOK; + if constexpr (kAllOK) { + if (object) { + auto* type = &object->TypeInfo(); + if constexpr (kHasDefaultCase) { + // Evaluate non-default cases. + if (!detail::NonDefaultCases(object, type, result, + traits::Slice<0, kDefaultIndex>(cases))) { + // Nothing matched. Evaluate default case. + if constexpr (kHasReturnType) { + new (result) RETURN_TYPE( + static_cast(std::get(cases)({}))); + } else { + std::get(cases)({}); + } + } + } else { + if (!detail::NonDefaultCases(object, type, result, std::move(cases))) { + // Nothing matched. No default case. + if constexpr (kHasReturnType) { + new (result) RETURN_TYPE(); + } } } } else { - detail::NonDefaultCases(object, type, result, std::move(cases)); - } - } else { - // Object is nullptr, so no cases can match - if constexpr (kHasDefaultCase) { - // Evaluate default case. - if constexpr (kHasReturnType) { - *result = static_cast(std::get(cases)({})); + // Object is nullptr, so no cases can match + if constexpr (kHasDefaultCase) { + // Evaluate default case. + if constexpr (kHasReturnType) { + new (result) + RETURN_TYPE(static_cast(std::get(cases)({}))); + } else { + std::get(cases)({}); + } } else { - std::get(cases)({}); + // No default case, no case can match. + if constexpr (kHasReturnType) { + new (result) RETURN_TYPE(); + } } } } @@ -760,9 +787,15 @@ inline auto Switch(T* object, CASES&&... cases) { static constexpr bool kHasReturnType = !std::is_same_v; if constexpr (kHasReturnType) { - ReturnType res = {}; - detail::SwitchCases(object, &res, std::forward_as_tuple(std::forward(cases)...)); - return res; + // Replacement for std::aligned_storage as this is broken on earlier versions of MSVC. + struct alignas(alignof(ReturnType)) ReturnStorage { + uint8_t data[sizeof(ReturnType)]; + }; + ReturnStorage storage; + auto* res = utils::Bitcast(&storage); + TINT_DEFER(res->~ReturnType()); + detail::SwitchCases(object, res, std::forward_as_tuple(std::forward(cases)...)); + return *res; } else { detail::SwitchCases(object, nullptr, std::forward_as_tuple(std::forward(cases)...)); diff --git a/src/tint/castable_test.cc b/src/tint/castable_test.cc index e1d29a441c..52efd80e9d 100644 --- a/src/tint/castable_test.cc +++ b/src/tint/castable_test.cc @@ -710,6 +710,32 @@ TEST(Castable, SwitchNullNoDefault) { EXPECT_TRUE(default_called); } +TEST(Castable, SwitchReturnNoDefaultConstructor) { + struct Object { + explicit Object(int v) : value(v) {} + int value; + }; + + std::unique_ptr frog = std::make_unique(); + { + auto result = Switch( + frog.get(), // + [](Mammal*) { return Object(1); }, // + [](Amphibian*) { return Object(2); }, // + [](Default) { return Object(3); }); + static_assert(std::is_same_v); + EXPECT_EQ(result.value, 2); + } + { + auto result = Switch( + frog.get(), // + [](Mammal*) { return Object(1); }, // + [](Default) { return Object(3); }); + static_assert(std::is_same_v); + EXPECT_EQ(result.value, 3); + } +} + // IsCastable static tests static_assert(IsCastable); static_assert(IsCastable);