On 20/09/17 18:59 +0100, Jonathan Wakely wrote: >On 20/09/17 17:52 +0100, Jonathan Wakely wrote: >>On 20/09/17 16:36 +0100, Jonathan Wakely wrote: >>>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 : >>>>>>2017-07-28 22:29 GMT+02:00 Daniel Krügler : >>>>>>>2017-07-28 22:25 GMT+02:00 Tim Song : >>>>>>>>On Fri, Jul 28, 2017 at 4:10 PM, Daniel Krügler >>>>>>>> wrote: >>>>>>>>>+ // Performs an implicit conversion from _Tp to __sv_type. >>>>>>>>>+ template >>>>>>>>>+ 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 >>>#include >>> >>>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. >> >>This patch fixes the failure above, I'm testing it now. >> >>--- a/libstdc++-v3/include/bits/basic_string.h >>+++ b/libstdc++-v3/include/bits/basic_string.h >>@@ -115,6 +115,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 >> template >> using _If_sv = enable_if_t< >> __and_, >>+ __not_>, >> __not_>>::value, >> _Res>; >> >> > >I'm committing this to fix the regression. If the final LWG 2946 >resolution does something different we can change it again. > >Tested powerpc64le-linux, committed to trunk. > >I'll start another test run with all variations ... That test run revealed I forgot to make the same change to the _If_sv helper in the old std::basic_string.