Re: [PATCH] libstdc++: Define C++20 range utilities and range factories

[Jonathan Wakely <jwakely at redhat dot com>]

On 19/11/19 08:51 +0000, Jonathan Wakely wrote:
> On 19/11/19 09:38 +0100, Stephan Bergmann wrote:
> > On 17/11/2019 02:07, Jonathan Wakely wrote:
> > > This adds another chunk of the <ranges> header.
> > >
> > > The changes from P1456R1 (Move-only views) and P1862R1 (Range adaptors
> > > for non-copyable iterators) are included, but not the changes from
> > > P1870R1 (forwarding-range<T> is too subtle).
> > >
> > > The tests for subrange and iota_view are poor and should be improved.
> >
> > At least with recent Clang trunk, with -std=c++2a this causes failures like
> >
> > > In file included from gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/10.0.0/../../../../include/c++/10.0.0/set:61:
> > > gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/10.0.0/../../../../include/c++/10.0.0/bits/stl_set.h:1057:48: error: default initialization of an object of const type 'const bool'
> > > template<typename _Tp> inline constexpr bool __enable_view_impl;
> > >                                              ^
> > > In file included from gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/10.0.0/../../../../include/c++/10.0.0/set:62:
> > > gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/10.0.0/../../../../include/c++/10.0.0/bits/stl_multiset.h:1045:48: error: redefinition of '__enable_view_impl'
> > > template<typename _Tp> inline constexpr bool __enable_view_impl;
> > >                                              ^
> > > gcc/trunk/inst/lib/gcc/x86_64-pc-linux-gnu/10.0.0/../../../../include/c++/10.0.0/bits/stl_set.h:1057:48: note: previous definition is here
> > > template<typename _Tp> inline constexpr bool __enable_view_impl;
> > >                                              ^
> >
> > and I'm not sure whether this is legal C++20 code that Clang is just 
> > not capable of handling?
>
> I'm not sure either. I checked with GCC and it did what I expected
> (the first declaration is not a definition, just a declaration). I'll
> check if that's valid.
>
> For now the simplest fix would be to just guard all those variable
> templates with #if __cpp_lib_concepts since std::ranges::enable_view
> only exists when concepts are supported (which is false for Clang).
>
> I'll look into it, thanks for the heads up.

GCC is wrong. I reported https://gcc.gnu.org/PR92576 and I've fixed the
libstdc++ code with the attached patch.

commit 60c1e565ca97887d1d51490237bc3463875c4e99
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Nov 19 09:13:42 2019 +0000

    libstdc++: Fix declarations of variable templates
    
    This code is invalid and rejected by other compilers (see PR 92576).
    
            * include/bits/regex.h (ranges::__detail::__enable_view_impl): Fix
            declaration.
            * include/bits/stl_multiset.h (ranges::__detail::__enable_view_impl):
            Likewise.
            * include/bits/stl_set.h (ranges::__detail::__enable_view_impl):
            Likewise.
            * include/bits/unordered_set.h (ranges::__detail::__enable_view_impl):
            Likewise.
            * include/debug/multiset.h (ranges::__detail::__enable_view_impl):
            Likewise.
            * include/debug/set.h (ranges::__detail::__enable_view_impl): Likewise.
            * include/debug/unordered_set (ranges::__detail::__enable_view_impl):
            Likewise.

diff --git a/libstdc++-v3/include/bits/regex.h b/libstdc++-v3/include/bits/regex.h
index 49994369563..4a19065fb58 100644
--- a/libstdc++-v3/include/bits/regex.h
+++ b/libstdc++-v3/include/bits/regex.h
@@ -2061,7 +2061,7 @@ _GLIBCXX_END_NAMESPACE_CXX11
 #if __cplusplus > 201703L
 namespace ranges::__detail
 {
-  template<typename _Tp> inline constexpr bool __enable_view_impl;
+  template<typename _Tp> extern inline const bool __enable_view_impl;
   template<typename _Bi_iter, typename _Alloc>
     inline constexpr bool __enable_view_impl<match_results<_Bi_iter, _Alloc>>
       = false;
diff --git a/libstdc++-v3/include/bits/stl_multiset.h b/libstdc++-v3/include/bits/stl_multiset.h
index 9e34961f4a5..14207e62205 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -1042,7 +1042,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
 #if __cplusplus > 201703L
 namespace ranges::__detail
 {
-  template<typename _Tp> inline constexpr bool __enable_view_impl;
+  template<typename _Tp> extern inline const bool __enable_view_impl;
   template<typename _Key, typename _Compare, typename _Alloc>
     inline constexpr bool
       __enable_view_impl<_GLIBCXX_STD_C::multiset<_Key, _Compare, _Alloc>>
diff --git a/libstdc++-v3/include/bits/stl_set.h b/libstdc++-v3/include/bits/stl_set.h
index 135d57af496..85f26c24dae 100644
--- a/libstdc++-v3/include/bits/stl_set.h
+++ b/libstdc++-v3/include/bits/stl_set.h
@@ -1054,7 +1054,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
 #if __cplusplus > 201703L
 namespace ranges::__detail
 {
-  template<typename _Tp> inline constexpr bool __enable_view_impl;
+  template<typename _Tp> extern inline const bool __enable_view_impl;
   template<typename _Key, typename _Compare, typename _Alloc>
     inline constexpr bool
       __enable_view_impl<_GLIBCXX_STD_C::set<_Key, _Compare, _Alloc>> = false;
diff --git a/libstdc++-v3/include/bits/unordered_set.h b/libstdc++-v3/include/bits/unordered_set.h
index 98943fb1a47..b138d02bbe8 100644
--- a/libstdc++-v3/include/bits/unordered_set.h
+++ b/libstdc++-v3/include/bits/unordered_set.h
@@ -1775,7 +1775,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
 #if __cplusplus > 201703L
 namespace ranges::__detail
 {
-  template<typename _Tp> inline constexpr bool __enable_view_impl;
+  template<typename _Tp> extern inline const bool __enable_view_impl;
   template<typename _Val, typename _Hash, typename _Eq, typename _Alloc>
     inline constexpr bool
       __enable_view_impl<_GLIBCXX_STD_C::unordered_set<_Val, _Hash, _Eq,
diff --git a/libstdc++-v3/include/debug/multiset.h b/libstdc++-v3/include/debug/multiset.h
index 00c5bcc0f24..62c7672dfb6 100644
--- a/libstdc++-v3/include/debug/multiset.h
+++ b/libstdc++-v3/include/debug/multiset.h
@@ -635,7 +635,7 @@ namespace __debug
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 namespace ranges::__detail
 {
-  template<typename _Tp> inline constexpr bool __enable_view_impl;
+  template<typename _Tp> extern inline const bool __enable_view_impl;
   template<typename _Key, typename _Compare, typename _Alloc>
     inline constexpr bool
       __enable_view_impl<std::__debug::multiset<_Key, _Compare, _Alloc>>
diff --git a/libstdc++-v3/include/debug/set.h b/libstdc++-v3/include/debug/set.h
index 16eee2947c9..8d0852a6af8 100644
--- a/libstdc++-v3/include/debug/set.h
+++ b/libstdc++-v3/include/debug/set.h
@@ -646,7 +646,7 @@ namespace __debug
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 namespace ranges::__detail
 {
-  template<typename _Tp> inline constexpr bool __enable_view_impl;
+  template<typename _Tp> extern inline const bool __enable_view_impl;
   template<typename _Key, typename _Compare, typename _Alloc>
     inline constexpr bool
       __enable_view_impl<std::__debug::set<_Key, _Compare, _Alloc>> = false;
diff --git a/libstdc++-v3/include/debug/unordered_set b/libstdc++-v3/include/debug/unordered_set
index 5dbb754a100..4b814cb20e0 100644
--- a/libstdc++-v3/include/debug/unordered_set
+++ b/libstdc++-v3/include/debug/unordered_set
@@ -1187,7 +1187,7 @@ namespace __debug
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 namespace ranges::__detail
 {
-  template<typename _Tp> inline constexpr bool __enable_view_impl;
+  template<typename _Tp> extern inline const bool __enable_view_impl;
   template<typename _Val, typename _Hash, typename _Eq, typename _Alloc>
     inline constexpr bool
       __enable_view_impl<std::__debug::unordered_set<_Val, _Hash, _Eq, _Alloc>>