Re: [PATCH] Use _GLIBCXX_NOEXCEPT_IF for std::swap

[Jonathan Wakely <jwakely at redhat dot com>]

On 02/05/19 21:33 +0100, Jonathan Wakely wrote:
> On 02/05/19 20:11 +0100, Jonathan Wakely wrote:
> > On 02/05/19 20:09 +0100, Jonathan Wakely wrote:
> > > On 02/05/19 20:34 +0200, Stephan Bergmann wrote:
> > > > On 29/04/2019 15:26, Jonathan Wakely wrote:
> > > > >     * include/bits/move.h (swap(T&, T&), swap(T (&)[N], T (&)[N])): Use
> > > > >     _GLIBCXX_NOEXCEPT_IF to simplify declarations.
> > > > >
> > > > > This just avoids having to repeat the name and parameter-list of the
> > > > > functions.
> > > >
> > > > libstdc++-v3/include/std/type_traits still has
> > > >
> > > > > template<typename _Tp>
> > > > > inline
> > > > > typename enable_if<__and_<__not_<__is_tuple_like<_Tp>>,
> > > > > 			      is_move_constructible<_Tp>,
> > > > > 			      is_move_assignable<_Tp>>::value>::type
> > > > > swap(_Tp&, _Tp&)
> > > > > noexcept(__and_<is_nothrow_move_constructible<_Tp>,
> > > > > 	            is_nothrow_move_assignable<_Tp>>::value);
> > > >
> > > > using noexcept instead of _GLIBCXX_NOEXPECT_IF, and at least 
> > > > during configure of building LibreOffice with Clang, that causes 
> > > > failure
> > > >
> > > > > .../gcc/trunk/lib/gcc/x86_64-pc-linux-gnu/10.0.0/../../../../include/c++/10.0.0/bits/move.h:185:5: error: exception specification in declaration does not match previous declaration
> > > > > swap(_Tp& __a, _Tp& __b)
> > > > > ^
> > > > > .../gcc/trunk/lib/gcc/x86_64-pc-linux-gnu/10.0.0/../../../../include/c++/10.0.0/type_traits:2531:5: note: previous declaration is here
> > > > > swap(_Tp&, _Tp&)
> > > > > ^
> > > >
> > > > I didn't try to track down under what conditions 
> > > > _GLIBCXX_NOEXCEPT_IF would not expand to noexcept, but I assume 
> > > > that just type_traits needs adapting, too?
> > >
> > > That's weird. _GLIBCXX_NOEXCEPT_IF is only empty for C++11, in which
> > > case the declaration in <type_traits> isn't seen anyway. And there's
> > > no point using _GLIBCXX_NOEXCEPT_IF in <type_traits> because we can
> > > (and should) just use noexcept directly in C++11 code. The macro
> > > exists for code that needs to be compiled as C++98 too.
> > >
> > > The only difference is that there's an extra set of parentheses around
> > > the NOEXCEPT_IF condition, so the preprocessor doesn't try to eat the
> > > comma.
> > >
> > > So one declaration is:
> > >
> > > template<typename _Tp>
> > > inline
> > > typename enable_if<__and_<__not_<__is_tuple_like<_Tp>>,
> > > 			      is_move_constructible<_Tp>,
> > > 			      is_move_assignable<_Tp>>::value>::type
> > > swap(_Tp&, _Tp&)
> > > noexcept(__and_<is_nothrow_move_constructible<_Tp>,
> > > 	            is_nothrow_move_assignable<_Tp>>::value);
> > >
> > > And the other is:
> > >
> > > template<typename _Tp>
> > > inline
> > > typename enable_if<__and_<__not_<__is_tuple_like<_Tp>>,
> > > 			      is_move_constructible<_Tp>,
> > > 			      is_move_assignable<_Tp>>::value>::type
> > > swap(_Tp&, _Tp&)
> > > noexcept((__and_<is_nothrow_move_constructible<_Tp>,
> > > 	            is_nothrow_move_assignable<_Tp>>::value));
> >
> > Yep, Clang doesn't like that:
> > https://wandbox.org/permlink/clslE9PGCVtKPppz
>
> This was also reported as https://gcc.gnu.org/PR90314

And here's what I've tested and committed.


commit 3a40e123af3226cd577de7db678fff5815816487
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu May 2 21:52:09 2019 +0100

    PR libstdc++/90314 fix non-equivalent declarations of std::swap
    
    In order to use the _GLIBCXX_NOEXCEPT_IF macro for an expression
    containing commas I enclosed it in parentheses, so the preprocessor
    wouldn't treat it as two arguments to the function-like macro. Clang
    gives an error because now the noexcept-specifier noexcept((C)) is not
    equivalent to the noexcept(C) one on the declaration of swap in
    <type_traits>.
    
    Instead of requiring extra parentheses around the expression, redefine
    _GLIBCXX_NOEXCEPT_IF as a variadic macro (even though supporting that in
    C++98 is a GNU extension).
    
            PR libstdc++/90314
            * include/bits/c++config (_GLIBCXX_NOEXCEPT_IF): Use variadic macro.
            * include/bits/move.h (swap): Remove extra parentheses.

diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index 5016f4853de..ca1557af564 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -157,12 +157,12 @@
 #ifndef _GLIBCXX_NOEXCEPT
 # if __cplusplus >= 201103L
 #  define _GLIBCXX_NOEXCEPT noexcept
-#  define _GLIBCXX_NOEXCEPT_IF(_COND) noexcept(_COND)
+#  define _GLIBCXX_NOEXCEPT_IF(...) noexcept(__VA_ARGS__)
 #  define _GLIBCXX_USE_NOEXCEPT noexcept
 #  define _GLIBCXX_THROW(_EXC)
 # else
 #  define _GLIBCXX_NOEXCEPT
-#  define _GLIBCXX_NOEXCEPT_IF(_COND)
+#  define _GLIBCXX_NOEXCEPT_IF(...)
 #  define _GLIBCXX_USE_NOEXCEPT throw()
 #  define _GLIBCXX_THROW(_EXC) throw(_EXC)
 # endif
diff --git a/libstdc++-v3/include/bits/move.h b/libstdc++-v3/include/bits/move.h
index 996078cfbce..7271e273e8e 100644
--- a/libstdc++-v3/include/bits/move.h
+++ b/libstdc++-v3/include/bits/move.h
@@ -183,8 +183,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     void
 #endif
     swap(_Tp& __a, _Tp& __b)
-    _GLIBCXX_NOEXCEPT_IF((__and_<is_nothrow_move_constructible<_Tp>,
-				 is_nothrow_move_assignable<_Tp>>::value))
+    _GLIBCXX_NOEXCEPT_IF(__and_<is_nothrow_move_constructible<_Tp>,
+				is_nothrow_move_assignable<_Tp>>::value)
     {
       // concept requirements
       __glibcxx_function_requires(_SGIAssignableConcept<_Tp>)