[PATCH] Fix return type detection in <variant> visit()

[PATCH] Fix return type detection in <variant> visit()

[Tim Shen via gcc-patches]

[-- Attachment #1: Type: text/plain, Size: 122 bytes --]

This is an obvious missing std::forward. :)

Testing on x86_64-linux-gnu, but I expect it to pass.

-- 
Regards,
Tim Shen

[-- Attachment #2: a.diff --]
[-- Type: text/x-patch, Size: 1593 bytes --]

commit 08235141a7e06db2b604b5869c9d8e4aaf8fa29b
Author: Tim Shen <timshen@google.com>
Date:   Tue Feb 14 13:55:18 2017 -0800

    2017-02-14  Tim Shen  <timshen@google.com>
    
            PR libstdc++/79513
            * include/std/variant (visit()): Forward variant types to the return
            type detection code.
            * testsuite/20_util/variant/compile.cc: Add test cases.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index c5138e56803..866c4c40a61 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1263,7 +1263,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	__throw_bad_variant_access("Unexpected index");
 
       using _Result_type =
-	decltype(std::forward<_Visitor>(__visitor)(get<0>(__variants)...));
+	decltype(std::forward<_Visitor>(__visitor)(
+	    get<0>(std::forward<_Variants>(__variants))...));
 
       constexpr auto& __vtable = __detail::__variant::__gen_vtable<
 	_Result_type, _Visitor&&, _Variants&&...>::_S_vtable;
diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc
index 65f4326c397..d40a4ccb784 100644
--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
@@ -291,6 +291,13 @@ void test_visit()
     };
     static_assert(visit(Visitor(), variant<int, nonliteral>(0)), "");
   }
+  // PR libstdc++/79513
+  {
+    std::variant<int> v(5);
+    std::visit([](int&){}, v);
+    std::visit([](int&&){}, std::move(v));
+    (void)v;
+  }
 }
 
 void test_constexpr()

Re: [PATCH] Fix return type detection in <variant> visit()

[Jonathan Wakely]

On 14/02/17 13:59 -0800, Tim Shen via libstdc++ wrote:
>This is an obvious missing std::forward. :)

I was about to look into it, I assumed it would be something simple!

>diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc
>index 65f4326c397..d40a4ccb784 100644
>--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
>+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
>@@ -291,6 +291,13 @@ void test_visit()
>     };
>     static_assert(visit(Visitor(), variant<int, nonliteral>(0)), "");
>   }
>+  // PR libstdc++/79513
>+  {
>+    std::variant<int> v(5);
>+    std::visit([](int&){}, v);
>+    std::visit([](int&&){}, std::move(v));
>+    (void)v;

Is this to suppress an unused variable warning?

If it is, please use an attribute instead, as it's more reliable:

    std::variant<int> v __attribute__((unused)) (5);

OK for trunk if testing passes, thanks.

Re: [PATCH] Fix return type detection in <variant> visit()

[Tim Shen via gcc-patches]

On Tue, Feb 14, 2017 at 2:49 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 14/02/17 13:59 -0800, Tim Shen via libstdc++ wrote:
>>
>> This is an obvious missing std::forward. :)
>
>
> I was about to look into it, I assumed it would be something simple!
>
>> diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc
>> b/libstdc++-v3/testsuite/20_util/variant/compile.cc
>> index 65f4326c397..d40a4ccb784 100644
>> --- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
>> +++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
>> @@ -291,6 +291,13 @@ void test_visit()
>>     };
>>     static_assert(visit(Visitor(), variant<int, nonliteral>(0)), "");
>>   }
>> +  // PR libstdc++/79513
>> +  {
>> +    std::variant<int> v(5);
>> +    std::visit([](int&){}, v);
>> +    std::visit([](int&&){}, std::move(v));
>> +    (void)v;
>
>
> Is this to suppress an unused variable warning?
>
> If it is, please use an attribute instead, as it's more reliable:
>
>    std::variant<int> v __attribute__((unused)) (5);

Even better, I used the shiny new [[gnu::unused]]. :)

>
> OK for trunk if testing passes, thanks.
>

Tested and committed.


-- 
Regards,
Tim Shen