From c41c3b7a575b8b14ac979993662f43aa90626512 Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Fri, 29 Jul 2022 06:50:34 -0600 Subject: [PATCH] Fix Variant::CastAndCall SFINAE for auto return type functors `Variant::CastAndCall` was using the C++11 style for an `auto` return where the return type was specified with a `->` that got the `decltype` of the return value of the functor. This was used as part of SFINAE to pick whether you needed the const or non-const version. However, this was causing a problem with functions that got an error when deducing the return type for that. This was particularly problematic for lambda functions. For example, say you have the following simple `CastAndCall`. ```cpp variant.CastAndCall([](auto& x){ ++x; }); ``` To determine the return type of the lambda (`void`), the function has to be compiled. But when it is compiled with a const type, which happens when deducing the const version of `CastAndCall`, you get a compile error. This error is not considered a substitution error (hence SFINAE), it is an outright error. So you get a compile error just trying to deduce the type. The solution was to move to the C++14 version of an auto return type. In this case, the return type is no longer important for SFINAE and is delayed until the function is actually compiled with the specific template parameters. This would be a problem if the const version of `CastAndCall` was used when the non-const version was needed. But now both versions will pass SFINAE and thus the non-const version will be chosen as long as the `Variant` object itself is non-const. If the `Variant` object itself is const, then that is in fact a legitimate error, so a compile error is OK. One thing I find wierd is that `CastAndCall` still has a `noexcept` expression that will likewise cause a compile error in this case. However, it is still working. I _think_ the difference is that `noexcept` is not used to determine template substitution/overloaded, so is therefore ignored until the function is actually compiled. --- vtkm/exec/internal/testing/UnitTestVariant.cxx | 2 ++ vtkm/internal/VariantImpl.h | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vtkm/exec/internal/testing/UnitTestVariant.cxx b/vtkm/exec/internal/testing/UnitTestVariant.cxx index 25dcfd32a..64834a369 100644 --- a/vtkm/exec/internal/testing/UnitTestVariant.cxx +++ b/vtkm/exec/internal/testing/UnitTestVariant.cxx @@ -444,6 +444,8 @@ void TestCastAndCall() variant1.CastAndCall(TestFunctorModify{}, 1); VTKM_TEST_ASSERT(variant1.Get<1>().Value == 2, "Variant object not updated."); + + variant1.CastAndCall([](auto& object) { ++object.Value; }); } void TestCopyInvalid() diff --git a/vtkm/internal/VariantImpl.h b/vtkm/internal/VariantImpl.h index 5533477cc..7e586b6de 100644 --- a/vtkm/internal/VariantImpl.h +++ b/vtkm/internal/VariantImpl.h @@ -510,7 +510,6 @@ public: template VTK_M_DEVICE auto CastAndCall(Functor&& f, Args&&... args) const noexcept(noexcept(f(std::declval&>(), args...))) - -> decltype(f(std::declval&>(), args...)) { VTKM_ASSERT(this->IsValid()); return detail::VariantCastAndCallImpl( @@ -521,7 +520,6 @@ public: VTK_M_DEVICE auto CastAndCall(Functor&& f, Args&&... args) noexcept(noexcept(f(std::declval&>(), args...))) - -> decltype(f(std::declval&>(), args...)) { VTKM_ASSERT(this->IsValid()); return detail::VariantCastAndCallImpl(