[committed] libstdc++: Use concepts and conditional explicit in std::optional

[Jonathan Wakely]

On Thu, 25 Jul 2024 at 19:55, François Dumont <frs.dumont@gmail.com> wrote:
>
>
> On 25/07/2024 20:24, Jonathan Wakely wrote:
> > On Thu, 25 Jul 2024 at 19:23, Jonathan Wakely <jwakely@redhat.com> wrote:
> >> On Thu, 25 Jul 2024 at 19:22, François Dumont <frs.dumont@gmail.com> wrote:
> >>>
> >>> On 25/07/2024 12:55, Jonathan Wakely wrote:
> >>>> Tested x86_64-linux. Pushed to trunk.
> >>>>
> >>>> -- >8 --
> >>>>
> >>>> For C++20 mode we can improve compile times by using conditional
> >>>> explicit to reduce the number of constructor overloads. We can also use
> >>>> requires-clauses instead of SFINAE to implement constraints on the
> >>>> constructors and assignment operators.
> >>>>
> >>>> libstdc++-v3/ChangeLog:
> >>>>
> >>>>        * include/std/optional (optional): Use C++20 features to
> >>>>        simplify overload sets for constructors and assignment
> >>>>        operators.
> >>>> ---
> >>>>    libstdc++-v3/include/std/optional | 130 +++++++++++++++++++++++++++---
> >>>>    1 file changed, 121 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
> >>>> index 700e7047aba..2cc0221865e 100644
> >>>> --- a/libstdc++-v3/include/std/optional
> >>>> +++ b/libstdc++-v3/include/std/optional
> >>>> @@ -768,6 +768,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >>>>            is_assignable<_Tp&, const optional<_Up>&&>,
> >>>>            is_assignable<_Tp&, optional<_Up>&&>>;
> >>>>
> >>>> +#if __cpp_concepts && __cpp_conditional_explicit && __glibcxx_remove_cvref
> >>>> +# define _GLIBCXX_USE_CONSTRAINTS_FOR_OPTIONAL 1
> >>>> +#endif
> >>>> +
> >>>>      /**
> >>>>        * @brief Class template for optional values.
> >>>>        */
> >>>> @@ -794,17 +798,37 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >>>>          using _Base = _Optional_base<_Tp>;
> >>>>
> >>>>          // SFINAE helpers
> >>>> -      template<typename _Up>
> >>>> -     using __not_self = __not_<is_same<optional, __remove_cvref_t<_Up>>>;
> >>>> -      template<typename _Up>
> >>>> -     using __not_tag = __not_<is_same<in_place_t, __remove_cvref_t<_Up>>>;
> >>>> -      template<typename... _Cond>
> >>>> -     using _Requires = enable_if_t<__and_v<_Cond...>, bool>;
> >>> Minor but above comment is an orphan now.
> >> Everything below it is a SFINAE helper, in both branches of the #if
> > But if that's not clear, maybe the comment does more harm than good
> > and should be removed.
> >
> I try to avoid blank line after a comment but it's clear for me now so
> as you wish.

I didn't want it to run into the _GLIBCXX_RESOLVE_LIB_DEFECTS comment
which is a separate comment (and doesn't apply to everything in both
branches).

I prefer to leave a blank line in many cases, otherwise it looks like
the comment only applies to the very next declaration.

e.g. this looks like the comment belongs to the first one only:

// constructors
foo()
{ }

foo(const foo&)
{ }

Whereas with a blank line after the comment it doesn't look like it
only applies to one. You can find this pattern throughout the library,
e.g. <tuple> line 1145, <expected> line 545, etc.