Bug 100384 - Compiling in c++17 mode breaks compilation of functions named visit()
Summary: Compiling in c++17 mode breaks compilation of functions named visit()
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 10.2.0
: P3 normal
Target Milestone: 9.5
Assignee: Not yet assigned to anyone
URL:
Keywords: rejects-valid
Depends on:
Blocks:
 
Reported: 2021-05-02 15:38 UTC by Albert Astals Cid
Modified: 2021-06-18 14:46 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-05-03 00:00:00


Attachments
main.cpp (203 bytes, text/plain)
2021-05-02 15:38 UTC, Albert Astals Cid
Details
error log (1.25 KB, text/plain)
2021-05-02 15:38 UTC, Albert Astals Cid
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Albert Astals Cid 2021-05-02 15:38:39 UTC
Created attachment 50733 [details]
main.cpp

The attached code fails to compile with 

g++ --std=c++17 main.cpp 

I've tried with gcc 11.1 too and it fails too.

To me it seems a bug in the implementation in which std::visit is leaking into the non std:: namespace, but maybe it's a bug in C++ specification?
Comment 1 Albert Astals Cid 2021-05-02 15:38:57 UTC
Created attachment 50734 [details]
error log
Comment 2 Albert Astals Cid 2021-05-03 12:59:16 UTC
People that know more C++ than me, told me

std::visit appears unconstrained to Variants...&& being actual std::variants (in case you inherited one, I guess), so std::visit, as an unconstrained perfect forwarder will win every overload set ranking. And std::visit is brought in by ADL via std::vector

it's a misfeature, this has been adopted in C++23 http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2162r2.html 

So I guess the proper resolution for this issue would be "not a bug", but will leave you to decide on it :)
Comment 3 Jonathan Wakely 2021-05-03 14:08:17 UTC
Not a bug.

PR 90943 already exists for the P2162R3 support.
Comment 4 Patrick Palka 2021-05-03 14:44:55 UTC
Hmm, isn't problem here ultimately that std::visit is specified to have a concrete SFINAE-friendly return type, but our implementation uses a deduced return type?
Comment 5 Jonathan Wakely 2021-05-03 19:51:55 UTC
Possibly, but it needs to be constrained for p2162 anyway, which I'm doing via the return type.
Comment 6 Jonathan Wakely 2021-05-03 20:02:59 UTC
Yes, we can fix this easily without the rest of P2162 (and can backport it):

--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1248,7 +1248,8 @@ namespace __variant
 #endif
 
   template<typename _Visitor, typename... _Variants>
-    constexpr decltype(auto) visit(_Visitor&&, _Variants&&...);
+    constexpr invoke_result_t<_Visitor, variant_alternative_t<0, _Variants>...>
+    visit(_Visitor&&, _Variants&&...);
 
   template<typename... _Types>
     inline enable_if_t<(is_move_constructible_v<_Types> && ...)
@@ -1736,7 +1737,7 @@ namespace __variant
 
 
   template<typename _Visitor, typename... _Variants>
-    constexpr decltype(auto)
+    constexpr invoke_result_t<_Visitor, variant_alternative_t<0, _Variants>...>
     visit(_Visitor&& __visitor, _Variants&&... __variants)
     {
       if ((__variants.valueless_by_exception() || ...))
Comment 7 GCC Commits 2021-05-04 11:19:17 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:af5b2b911dd80ae9cc87404b7e7ab807cf6655d4

commit r12-436-gaf5b2b911dd80ae9cc87404b7e7ab807cf6655d4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 4 12:16:46 2021 +0100

    libstdc++: Do not use deduced return type for std::visit [PR 100384]
    
    This avoids errors outside the immediate context when std::visit is an
    overload candidate because of ADL, but not actually viable.
    
    The solution is to give std::visit a non-deduced return type. New
    helpers are introduced for that, and existing ones refactored slightly.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/100384
            * include/std/variant (__get_t): New alias template yielding the
            return type of std::get<N> on a variant.
            (__visit_result_t): New alias template yielding the result of
            std::visit.
            (__same_types): Move into namespace __detail::__variant.
            (__check_visitor_results): Likewise. Use __invoke_result_t and
            __get_t.
            (__check_visitor_result): Remove.
            (visit): Use __visit_result_t for return type.
            * testsuite/20_util/variant/100384.cc: New test.
Comment 8 GCC Commits 2021-05-06 13:06:11 UTC
The releases/gcc-11 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:e99763ee6da8e378073b847243d9ac2538903534

commit r11-8363-ge99763ee6da8e378073b847243d9ac2538903534
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 4 12:16:46 2021 +0100

    libstdc++: Do not use deduced return type for std::visit [PR 100384]
    
    This avoids errors outside the immediate context when std::visit is an
    overload candidate because of ADL, but not actually viable.
    
    The solution is to give std::visit a non-deduced return type. New
    helpers are introduced for that, and existing ones refactored slightly.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/100384
            * include/std/variant (__get_t): New alias template yielding the
            return type of std::get<N> on a variant.
            (__visit_result_t): New alias template yielding the result of
            std::visit.
            (__same_types): Move into namespace __detail::__variant.
            (__check_visitor_results): Likewise. Use __invoke_result_t and
            __get_t.
            (__check_visitor_result): Remove.
            (visit): Use __visit_result_t for return type.
            * testsuite/20_util/variant/100384.cc: New test.
    
    (cherry picked from commit af5b2b911dd80ae9cc87404b7e7ab807cf6655d4)
Comment 9 GCC Commits 2021-06-18 12:56:30 UTC
The releases/gcc-10 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:8ad4d9b46944db1be6f1b216b5b8e74bd9f66937

commit r10-9937-g8ad4d9b46944db1be6f1b216b5b8e74bd9f66937
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 4 12:16:46 2021 +0100

    libstdc++: Do not use deduced return type for std::visit [PR 100384]
    
    This avoids errors outside the immediate context when std::visit is an
    overload candidate because of ADL, but not actually viable.
    
    The solution is to give std::visit a non-deduced return type. New
    helpers are introduced for that, and existing ones refactored slightly.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/100384
            * include/std/variant (__get_t): New alias template yielding the
            return type of std::get<N> on a variant.
            (__visit_result_t): New alias template yielding the result of
            std::visit.
            (__same_types): Move into namespace __detail::__variant.
            (__check_visitor_results): Likewise. Use __invoke_result_t and
            __get_t.
            (__check_visitor_result): Remove.
            (visit): Use __visit_result_t for return type.
            * testsuite/20_util/variant/100384.cc: New test.
    
    (cherry picked from commit af5b2b911dd80ae9cc87404b7e7ab807cf6655d4)
Comment 10 GCC Commits 2021-06-18 14:43:41 UTC
The releases/gcc-9 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:91d29ed563bd7e787921f997ea2f80cd87ee59b2

commit r9-9592-g91d29ed563bd7e787921f997ea2f80cd87ee59b2
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 4 12:16:46 2021 +0100

    libstdc++: Do not use deduced return type for std::visit [PR 100384]
    
    This avoids errors outside the immediate context when std::visit is an
    overload candidate because of ADL, but not actually viable.
    
    The solution is to give std::visit a non-deduced return type. New
    helpers are introduced for that.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/100384
            * include/std/variant (__get_t): New alias template yielding the
            return type of std::get<N> on a variant.
            (__visit_result_t): New alias template yielding the result of
            std::visit.
            (__do_visit): Use __get_t.
            (visit): Use __visit_result_t for return type.
            * testsuite/20_util/variant/100384.cc: New test.
    
    (cherry picked from commit af5b2b911dd80ae9cc87404b7e7ab807cf6655d4)
Comment 11 Jonathan Wakely 2021-06-18 14:46:39 UTC
Fixed for 9.5, 10.4 and 11.2