Re: Patch ping (was Re: [PATCH] libstdc++: Clear padding bits in atomic compare_exchange)

[Jonathan Wakely <jwakely@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2196 bytes --]

On Fri, 9 Sept 2022 at 20:01, Thomas Rodgers wrote:
>
> s/__weak/__is_weak/g  perhaps?

Yes, that'll do. Fixed by the attached, with a test to avoid it happening again.

Tested x86_64-linux, pushed to trunk.




>
> On Fri, Sep 9, 2022 at 11:46 AM Iain Sandoe via Libstdc++ <libstdc++@gcc.gnu.org> wrote:
>>
>>
>>
>> > On 9 Sep 2022, at 19:36, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:
>> >
>>
>> >> Here's a complete patch that combines the various incremental patches
>> >> that have been going around. I'm testing this now.
>> >>
>> >> Please take a look.
>> >
>> > unfortunately, this patch broke macOS bootstrap (seen on
>> > x86_64-apple-darwin11.4.2):
>> >
>> > In file included from /var/gcc/regression/master/10.7-gcc/build/x86_64-apple-darwin11.4.2/libstdc++-v3/include/bits/shared_ptr_atomic.h:33,
>> >                 from /var/gcc/regression/master/10.7-gcc/build/x86_64-apple-darwin11.4.2/libstdc++-v3/include/memory:78,
>> >                 from /vol/gcc/src/hg/master/darwin/libstdc++-v3/include/precompiled/stdc++.h:82:
>> > /var/gcc/regression/master/10.7-gcc/build/x86_64-apple-darwin11.4.2/libstdc++-v3/include/bits/atomic_base.h: In function 'bool std::__atomic_impl::__compare_exchange(_Tp&, _Val<_Tp>&, _Val<_Tp>&, bool, std::memory_order, std::memory_order)':
>> > /var/gcc/regression/master/10.7-gcc/build/x86_64-apple-darwin11.4.2/libstdc++-v3/include/bits/atomic_base.h:1008:49: error: expected primary-expression before ',' token
>> > 1008 |                                           __weak, int(__s), int(__f)))
>> >      |                                                 ^
>> > /var/gcc/regression/master/10.7-gcc/build/x86_64-apple-darwin11.4.2/libstdc++-v3/include/bits/atomic_base.h:1017:50: error: expected primary-expression before ',' token
>> > 1017 |                                            __weak, int(__s), int(__f));
>> >      |                                                  ^
>> >
>> > Darwin gcc predefines __weak= in gcc/config/darwin-c.cc (darwin_cpp_builtins).
>>
>> yes, __weak and __strong are Objective C things (in principle, applicable to non-Darwin targets
>> using NeXT runtime - there is at least one such target).
>>
>> Iain
>>

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2311 bytes --]

commit 007680f946eaffa3c6321624129e1ec18e673091
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Sep 9 21:03:58 2022

    libstdc++: Rename parameter to avoid darwin __weak qualifier
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/atomic_base.h (__atomic_impl::__compare_exchange):
            Rename __weak to __is_weak.
            * testsuite/17_intro/names.cc: Add __weak and __strong.

diff --git a/libstdc++-v3/include/bits/atomic_base.h b/libstdc++-v3/include/bits/atomic_base.h
index 29315547aab..6ea3268fdf0 100644
--- a/libstdc++-v3/include/bits/atomic_base.h
+++ b/libstdc++-v3/include/bits/atomic_base.h
@@ -990,7 +990,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     template<typename _Tp>
       _GLIBCXX_ALWAYS_INLINE bool
       __compare_exchange(_Tp& __val, _Val<_Tp>& __e, _Val<_Tp>& __i,
-			 bool __weak, memory_order __s, memory_order __f) noexcept
+			 bool __is_weak,
+			 memory_order __s, memory_order __f) noexcept
       {
 	__glibcxx_assert(__is_valid_cmpexch_failure_order(__f));
 
@@ -1005,7 +1006,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    __atomic_impl::__clear_padding(*__exp);
 	    if (__atomic_compare_exchange(std::__addressof(__val), __exp,
 					  __atomic_impl::__clear_padding(__i),
-					  __weak, int(__s), int(__f)))
+					  __is_weak, int(__s), int(__f)))
 	      return true;
 	    __builtin_memcpy(std::__addressof(__e), __exp, sizeof(_Vp));
 	    return false;
@@ -1014,7 +1015,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return __atomic_compare_exchange(std::__addressof(__val),
 					   std::__addressof(__e),
 					   std::__addressof(__i),
-					   __weak, int(__s), int(__f));
+					   __is_weak, int(__s), int(__f));
       }
   } // namespace __atomic_impl
 
diff --git a/libstdc++-v3/testsuite/17_intro/names.cc b/libstdc++-v3/testsuite/17_intro/names.cc
index ede2fe8caa7..86fb8f8999b 100644
--- a/libstdc++-v3/testsuite/17_intro/names.cc
+++ b/libstdc++-v3/testsuite/17_intro/names.cc
@@ -129,6 +129,10 @@
 // This clashes with newlib so don't use it.
 # define __lockable		cannot be used as an identifier
 
+#ifndef __APPLE__
+#define __weak   predefined qualifier on darwin
+#define __strong predefined qualifier on darwin
+#endif
 
 // Common template parameter names
 #define OutputIterator		OutputIterator is not a reserved name