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

[Jonathan Wakely]

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!

This change causes two regressions in C++17 mode, see
https://gcc.gnu.org/ml/gcc-testresults/2017-09/msg01674.html

FAIL: 21_strings/basic_string/cons/char/moveable2.cc execution test
FAIL: 21_strings/basic_string/cons/wchar_t/moveable2.cc execution test

Here's a reduced version of that test, which passes in C++14 and fails
in C++17:

#include <string>
#include <assert.h>

class tstring : public std::string
{
public:
  tstring() : std::string() {}
  tstring(tstring&& s) : std::string(std::move(s)) {}
};

int main()
{
  tstring b;
  b.push_back('1');
  tstring c(std::move(b));
  assert( c.size() == 1 && c[0] == '1' );
  assert( b.size() == 0 );
}

The second assertion fails, because this mem-initializer:

  tstring(tstring&& s) : std::string(std::move(s)) {}

now prefers to use the new constructor:

  basic_string(const _Tp& __t, const _Alloc& __a = _Alloc())

because tstring is convertible to string_view.

This turns a non-allocating move into an allocating copy.