On Fri, 13 Sept 2024 at 11:26, Jonathan Wakely <jwak...@redhat.com> wrote:
>
> On Sat, 3 Aug 2024 at 00:10, Jonathan Wakely <jwak...@redhat.com> wrote:
> >
> > On Fri, 2 Aug 2024 at 23:49, Giuseppe D'Angelo
> > <giuseppe.dang...@kdab.com> wrote:
> > >
> > > Hello,
> > >
> > > as usual thank you for the review. V2 is attached.
> > >
> > > On 02/08/2024 14:38, Jonathan Wakely wrote:
> > > > On Fri, 2 Aug 2024 at 13:17, Jonathan Wakely <jwak...@redhat.com> wrote:
> > > >>
> > > >> On Fri, 2 Aug 2024 at 11:45, Giuseppe D'Angelo wrote:
> > > >>>
> > > >>> Hello,
> > > >>>
> > > >>> The attached patch adds support for P2248R8 + P3217R0 (Enabling
> > > >>> list-initialization for algorithms, C++26). The big question is
> > > >>> whether
> > > >>> this keeps the code readable enough without introducing too much
> > > >>> #ifdef-ery, so any feedback is appreciated.
> > > >>
> > > >> Putting the extra args on the algorithmfwd.h declarations is a nice
> > > >> way to avoid any clutter on the definitions. I think that is very
> > > >> readable.
> > > >> Another option would be to not touch those forward declarations, but
> > > >> add new ones with the defaults:
> > > >>
> > > >> #if __glibcxx_default_template_type_for_algorithm_values
> > > >> // new declarations with default template args ...
> > > >> #endif
> > > >>
> > > >> But I think what you've done is good.
> > >
> > > I'll keep it then :)
> > >
> > > >> For ranges_algo.h I'm almost tempted to say we should just treat this
> > > >> as a DR, to avoid the #ifdef-ery. Almost.
> > > >> Is there any reason we can't rearrange the template parameters fo
> > > >> C++20 and C++23 mode? I don't think users are allowed to use explicit
> > > >> template argument lists for invoke e.g. ranges::find.operator()<Iter,
> > > >> Proj> so it should be unobservable if we change the order for C++20
> > > >> (and just don't add the default args until C++26). That might reduce
> > > >> the differences to just a line or two for each CPO.
> > >
> > > Indeed, users cannot rely on any specific order of the template
> > > arguments when calling algorithms. This is
> > >
> > > https://eel.is/c++draft/algorithms.requirements#15
> > >
> > > which has this note:
> > >
> > > "Consequently, an implementation can declare an algorithm with different
> > > template parameters than those presented"
> > >
> > > which of course does apply here: it's why P2248 could do these changes
> > > to begin with. The only reason why I kept them in the original order was
> > > a matter of caution, but sure, in the new patch I've changed them
> > > unconditionally and just used a macro to hide the default in pre-C++26
> > > modes. This should keep the code clean(er).
> > >
> > >
> > > > The merged wording also removes the redundant 'typename' from the
> > > > default arguments, but I think we might still need that for Clang
> > > > compat. I'm not sure when Clang fully implemented "down with
> > > > typename", but it was still causing issues some time last year.
> > >
> > > I hope it's fine if I keep it.
> >
> > Yes, certainly.
> >
> > This looks good to me now, thanks.
> >
> > Reviewed-by: Jonathan Wakely <jwak...@redhat.com>
> >
> > Patrick, could you please review + test this some time next week? If
> > it's all fine and you're happy with it too, please push to trunk.
>
> Looks like this wasn't applied. I've rebased it, but the new
> 23_containers/default_template_value.cc test fails.
>
> The problem is that all the std::erase overloads for containers have this
> added:
>
> template<typename _CharT, typename _Traits, typename _Alloc, typename _Up
> _GLIBCXX26_ALGORITHM_DEFAULT_VALUE_TYPE(_CharT)>
>
> But the definition of that macro expects an iterator, not the value type.
>
> Was the new test actually run?
>
> I've attached the rebased patch, which needs fixes for the std::erase
> overloads.
And this incremental diff fixes the std::erase problems.
diff --git a/libstdc++-v3/include/bits/stl_iterator_base_types.h
b/libstdc++-v3/include/bits/stl_iterator_base_types.h
index 2b87324eb77..f3a49fcc922 100644
--- a/libstdc++-v3/include/bits/stl_iterator_base_types.h
+++ b/libstdc++-v3/include/bits/stl_iterator_base_types.h
@@ -270,9 +270,11 @@ _GLIBCXX_END_NAMESPACE_VERSION
} // namespace
#if __glibcxx_algorithm_default_value_type // C++ >= 26
+# define _GLIBCXX26_DEFAULT_VALUE_TYPE(T) = T
# define _GLIBCXX26_ALGORITHM_DEFAULT_VALUE_TYPE(_Iterator) \
= typename iterator_traits<_Iterator>::value_type
#else
+# define _GLIBCXX26_DEFAULT_VALUE_TYPE(T)
# define _GLIBCXX26_ALGORITHM_DEFAULT_VALUE_TYPE(_It)
#endif
diff --git a/libstdc++-v3/include/std/deque b/libstdc++-v3/include/std/deque
index 48d2e308bdd..c58578d249e 100644
--- a/libstdc++-v3/include/std/deque
+++ b/libstdc++-v3/include/std/deque
@@ -118,7 +118,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename _Tp, typename _Alloc, typename _Up
- _GLIBCXX26_ALGORITHM_DEFAULT_VALUE_TYPE(_Tp)>
+ _GLIBCXX26_DEFAULT_VALUE_TYPE(_Tp)>
inline typename deque<_Tp, _Alloc>::size_type
erase(deque<_Tp, _Alloc>& __cont, const _Up& __value)
{
diff --git a/libstdc++-v3/include/std/forward_list
b/libstdc++-v3/include/std/forward_list
index d03c965e82a..25724b51c9e 100644
--- a/libstdc++-v3/include/std/forward_list
+++ b/libstdc++-v3/include/std/forward_list
@@ -77,7 +77,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ return __cont.remove_if(__pred); }
template<typename _Tp, typename _Alloc, typename _Up
- _GLIBCXX26_ALGORITHM_DEFAULT_VALUE_TYPE(_Tp)>
+ _GLIBCXX26_DEFAULT_VALUE_TYPE(_Tp)>
inline typename forward_list<_Tp, _Alloc>::size_type
erase(forward_list<_Tp, _Alloc>& __cont, const _Up& __value)
{
diff --git a/libstdc++-v3/include/std/list b/libstdc++-v3/include/std/list
index e26ca11c468..2331ee95e79 100644
--- a/libstdc++-v3/include/std/list
+++ b/libstdc++-v3/include/std/list
@@ -101,7 +101,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ return __cont.remove_if(__pred); }
template<typename _Tp, typename _Alloc, typename _Up
- _GLIBCXX26_ALGORITHM_DEFAULT_VALUE_TYPE(_Tp)>
+ _GLIBCXX26_DEFAULT_VALUE_TYPE(_Tp)>
inline typename list<_Tp, _Alloc>::size_type
erase(list<_Tp, _Alloc>& __cont, const _Up& __value)
{
diff --git a/libstdc++-v3/include/std/string b/libstdc++-v3/include/std/string
index 461bf6450ae..1e206917675 100644
--- a/libstdc++-v3/include/std/string
+++ b/libstdc++-v3/include/std/string
@@ -107,7 +107,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename _CharT, typename _Traits, typename _Alloc, typename _Up
- _GLIBCXX26_ALGORITHM_DEFAULT_VALUE_TYPE(_CharT)>
+ _GLIBCXX26_DEFAULT_VALUE_TYPE(_CharT)>
_GLIBCXX20_CONSTEXPR
inline typename basic_string<_CharT, _Traits, _Alloc>::size_type
erase(basic_string<_CharT, _Traits, _Alloc>& __cont, const _Up& __value)
diff --git a/libstdc++-v3/include/std/vector b/libstdc++-v3/include/std/vector
index 60a21bd92d1..6b12689cec8 100644
--- a/libstdc++-v3/include/std/vector
+++ b/libstdc++-v3/include/std/vector
@@ -131,7 +131,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename _Tp, typename _Alloc, typename _Up
- _GLIBCXX26_ALGORITHM_DEFAULT_VALUE_TYPE(_Tp)>
+ _GLIBCXX26_DEFAULT_VALUE_TYPE(_Tp)>
_GLIBCXX20_CONSTEXPR
inline typename vector<_Tp, _Alloc>::size_type
erase(vector<_Tp, _Alloc>& __cont, const _Up& __value)
