Re: [PATCH] PR libstdc++/79162 ambiguity in string assignment due to string_view overload (LWG 2946)

[Jonathan Wakely <jwakely at redhat dot com>]

On 04/09/17 16:48 +0100, Jonathan Wakely wrote:
> On 30/07/17 15:01 +0200, Daniel Krügler wrote:
> > 2017-07-28 22:40 GMT+02:00 Daniel Krügler <daniel.kruegler@gmail.com>:
> > > 2017-07-28 22:29 GMT+02:00 Daniel Krügler <daniel.kruegler@gmail.com>:
> > > > 2017-07-28 22:25 GMT+02:00 Tim Song <t.canens.cpp@gmail.com>:
> > > > > On Fri, Jul 28, 2017 at 4:10 PM, Daniel Krügler
> > > > > <daniel.kruegler@gmail.com> wrote:
> > > > > > +      // Performs an implicit conversion from _Tp to __sv_type.
> > > > > > +      template<typename _Tp>
> > > > > > +        static __sv_type _S_to_string_view(const _Tp& __svt)
> > > > > > +        {
> > > > > > +          return __svt;
> > > > > > +        }
> > > > >
> > > > > I might have gone for
> > > > >
> > > > > +        static __sv_type _S_to_string_view(__sv_type __svt) noexcept
> > > > > +        {
> > > > > +          return __svt;
> > > > > +        }
> > > > >
> > > > > With that, we can also use noexcept(_S_to_string_view(__t)) to make up
> > > > > for the absence of is_nothrow_convertible (basically the same thing I
> > > > > did in LWG 2993's PR).
> > > >
> > > > Agreed, that makes very much sense. I will adjust the P/R, but before
> > > > I resubmit I would like to get feedback whether the other two compare
> > > > functions also should become conditionally noexcept.
> > >
> > > Locally I have now performed the sole change of the _S_to_string_view
> > > declaration getting rid of the template, but would also like to gather
> > > feedback from the maintainers whether I should also change the form of
> > > the conditional noexcept to use the expression
> > >
> > > noexcept(_S_to_string_view(__t))
> > >
> > > instead of the current
> > >
> > > is_same<_Tp, __sv_type>::value
> > >
> > > as suggested by Tim Song.
> > >
> > > I'm asking also, because I have a paper proposing to standardize
> > > is_nothrow_convertible submitted for the upcoming C++ mailing - This
> > > would be one of the first applications in the library ;-)
> >
> > A slightly revised patch update: It replaces the _S_to_string_view
> > template by a simpler _S_to_string_view function as of Tim Song's
> > suggestion, but still uses the simplified noexcept specification
> > deferring it to a future application case for is_nothrow_convertible.
> > Furthermore now all three compare function templates are now
> > (conditionally) noexcept by an (off-list) suggestion from Jonathan
> > Wakely.
>
> I've committed this, after some whitespace fixes and testing.
>
> Thanks!


We also need this tweak, to account for the fact that the old
std::string has this signature:

     basic_string& replace(iterator __i1, iterator __i2,
			    initializer_list<_CharT> __l)


Tested powerpc64le-linux, committed to trunk.


commit c435fd7ed919ae54fc8f730844c7466822787ff8
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Sep 11 17:29:34 2017 +0100

    Adjust test to pass with old std::string
    
            * testsuite/21_strings/basic_string/lwg2946.cc: Adjust for
            compatibility with old COW std::string.

diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/lwg2946.cc b/libstdc++-v3/testsuite/21_strings/basic_string/lwg2946.cc
index 74d5a5c89a7..fe1f15553fb 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/lwg2946.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/lwg2946.cc
@@ -29,7 +29,7 @@ int main()
   s.assign({"abc", 1});
   s.insert(0, {"abc", 1});
   s.replace(0, 1, {"abc", 1});
-  s.replace(s.cbegin(), s.cbegin(), {"abc", 1});
+  s.replace(s.begin(), s.begin(), {"abc", 1});
   s.find({"abc", 1});
   s.rfind({"abc", 1});
   s.find_first_of({"abc", 1});