Re: [PATCH] libstdc++: mark variables as possibly unused

Jonathan Wakely via Gcc-patches Thu, 10 Sep 2020 08:23:24 -0700

On 28/08/20 08:40 +0200, Krystian Kuźniarek via Libstdc++ wrote:

Test suite confirmation:
All tests pass. Tested on both Manjaro and some Ubuntu 18.04 with gcc10

and gcc8 respectively.


Thanks.

Jonathan, one more thing. I hope it's what you asked for cause all I did
was:
make bootstrap


There has been no need to say "make bootstrap" for 10 years or so,
just "make" does a bootstrap.

make check


That runs all the tests for the entire compiler. What we're interested
in is whether there are any (new) failures caused by your patch,
specifically in the libstdc++ testsuite.

N.B. This patch is less relevant now, because these variables are
always used, because the __glibcxx_assert macro now expands to a
compile-time test (as long as the compiler supports
__builtin_is_constant_evaluated(), which GCC does).

But adding the attributes anyway is harmless, so I'll go ahead and
apply this patch. Thanks again.

On Fri, 28 Aug 2020 at 08:32, Krystian Kuźniarek <
krystian.kuznia...@gmail.com> wrote:

Ok, so here it is.
New diff:

diff --git a/libstdc++-v3/include/bits/atomic_base.h 
b/libstdc++-v3/include/bits/atomic_base.h
index 015acef83c4..c4a763aae5c 100644
--- a/libstdc++-v3/include/bits/atomic_base.h
+++ b/libstdc++-v3/include/bits/atomic_base.h
@@ -231,7 +231,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     _GLIBCXX_ALWAYS_INLINE void
     clear(memory_order __m = memory_order_seq_cst) noexcept
     {
-      memory_order __b = __m & __memory_order_mask;
+      memory_order __b __attribute__ ((__unused__))
+        = __m & __memory_order_mask;
       __glibcxx_assert(__b != memory_order_consume);
       __glibcxx_assert(__b != memory_order_acquire);
       __glibcxx_assert(__b != memory_order_acq_rel);
@@ -242,7 +243,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     _GLIBCXX_ALWAYS_INLINE void
     clear(memory_order __m = memory_order_seq_cst) volatile noexcept
     {
-      memory_order __b = __m & __memory_order_mask;
+      memory_order __b __attribute__ ((__unused__))
+        = __m & __memory_order_mask;
       __glibcxx_assert(__b != memory_order_consume);
       __glibcxx_assert(__b != memory_order_acquire);
       __glibcxx_assert(__b != memory_order_acq_rel);
@@ -416,7 +418,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _GLIBCXX_ALWAYS_INLINE void
       store(__int_type __i, memory_order __m = memory_order_seq_cst) noexcept
       {
-       memory_order __b = __m & __memory_order_mask;
+       memory_order __b __attribute__ ((__unused__))
+         = __m & __memory_order_mask;
        __glibcxx_assert(__b != memory_order_acquire);
        __glibcxx_assert(__b != memory_order_acq_rel);
        __glibcxx_assert(__b != memory_order_consume);
@@ -428,7 +431,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       store(__int_type __i,
            memory_order __m = memory_order_seq_cst) volatile noexcept
       {
-       memory_order __b = __m & __memory_order_mask;
+       memory_order __b __attribute__ ((__unused__))
+         = __m & __memory_order_mask;
        __glibcxx_assert(__b != memory_order_acquire);
        __glibcxx_assert(__b != memory_order_acq_rel);
        __glibcxx_assert(__b != memory_order_consume);
@@ -439,7 +443,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _GLIBCXX_ALWAYS_INLINE __int_type
       load(memory_order __m = memory_order_seq_cst) const noexcept
       {
-       memory_order __b = __m & __memory_order_mask;
+       memory_order __b __attribute__ ((__unused__))
+         = __m & __memory_order_mask;
        __glibcxx_assert(__b != memory_order_release);
        __glibcxx_assert(__b != memory_order_acq_rel);

@@ -449,7 +454,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _GLIBCXX_ALWAYS_INLINE __int_type
       load(memory_order __m = memory_order_seq_cst) const volatile noexcept
       {
-       memory_order __b = __m & __memory_order_mask;
+       memory_order __b __attribute__ ((__unused__))
+         = __m & __memory_order_mask;
        __glibcxx_assert(__b != memory_order_release);
        __glibcxx_assert(__b != memory_order_acq_rel);

@@ -475,8 +481,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       compare_exchange_weak(__int_type& __i1, __int_type __i2,
                            memory_order __m1, memory_order __m2) noexcept
       {
-       memory_order __b2 = __m2 & __memory_order_mask;
-       memory_order __b1 = __m1 & __memory_order_mask;
+       memory_order __b2 __attribute__ ((__unused__))
+         = __m2 & __memory_order_mask;
+       memory_order __b1 __attribute__ ((__unused__))
+         = __m1 & __memory_order_mask;
        __glibcxx_assert(__b2 != memory_order_release);
        __glibcxx_assert(__b2 != memory_order_acq_rel);
        __glibcxx_assert(__b2 <= __b1);
@@ -490,8 +498,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                            memory_order __m1,
                            memory_order __m2) volatile noexcept
       {
-       memory_order __b2 = __m2 & __memory_order_mask;
-       memory_order __b1 = __m1 & __memory_order_mask;
+       memory_order __b2 __attribute__ ((__unused__))
+         = __m2 & __memory_order_mask;
+       memory_order __b1 __attribute__ ((__unused__))
+         = __m1 & __memory_order_mask;
        __glibcxx_assert(__b2 != memory_order_release);
        __glibcxx_assert(__b2 != memory_order_acq_rel);
        __glibcxx_assert(__b2 <= __b1);
@@ -520,8 +530,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       compare_exchange_strong(__int_type& __i1, __int_type __i2,
                              memory_order __m1, memory_order __m2) noexcept
       {
-       memory_order __b2 = __m2 & __memory_order_mask;
-       memory_order __b1 = __m1 & __memory_order_mask;
+       memory_order __b2 __attribute__ ((__unused__))
+         = __m2 & __memory_order_mask;
+       memory_order __b1 __attribute__ ((__unused__))
+         = __m1 & __memory_order_mask;
        __glibcxx_assert(__b2 != memory_order_release);
        __glibcxx_assert(__b2 != memory_order_acq_rel);
        __glibcxx_assert(__b2 <= __b1);
@@ -535,8 +547,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                              memory_order __m1,
                              memory_order __m2) volatile noexcept
       {
-       memory_order __b2 = __m2 & __memory_order_mask;
-       memory_order __b1 = __m1 & __memory_order_mask;
+       memory_order __b2 __attribute__ ((__unused__))
+         = __m2 & __memory_order_mask;
+       memory_order __b1 __attribute__ ((__unused__))
+         = __m1 & __memory_order_mask;

        __glibcxx_assert(__b2 != memory_order_release);
        __glibcxx_assert(__b2 != memory_order_acq_rel);
@@ -736,7 +750,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       store(__pointer_type __p,
            memory_order __m = memory_order_seq_cst) noexcept
       {
-        memory_order __b = __m & __memory_order_mask;
+        memory_order __b __attribute__ ((__unused__))
+          = __m & __memory_order_mask;

        __glibcxx_assert(__b != memory_order_acquire);
        __glibcxx_assert(__b != memory_order_acq_rel);
@@ -749,7 +764,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       store(__pointer_type __p,
            memory_order __m = memory_order_seq_cst) volatile noexcept
       {
-       memory_order __b = __m & __memory_order_mask;
+       memory_order __b __attribute__ ((__unused__))
+         = __m & __memory_order_mask;
        __glibcxx_assert(__b != memory_order_acquire);
        __glibcxx_assert(__b != memory_order_acq_rel);
        __glibcxx_assert(__b != memory_order_consume);
@@ -760,7 +776,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _GLIBCXX_ALWAYS_INLINE __pointer_type
       load(memory_order __m = memory_order_seq_cst) const noexcept
       {
-       memory_order __b = __m & __memory_order_mask;
+       memory_order __b __attribute__ ((__unused__))
+         = __m & __memory_order_mask;
        __glibcxx_assert(__b != memory_order_release);
        __glibcxx_assert(__b != memory_order_acq_rel);

@@ -770,7 +787,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _GLIBCXX_ALWAYS_INLINE __pointer_type
       load(memory_order __m = memory_order_seq_cst) const volatile noexcept
       {
-       memory_order __b = __m & __memory_order_mask;
+       memory_order __b __attribute__ ((__unused__))
+         = __m & __memory_order_mask;
        __glibcxx_assert(__b != memory_order_release);
        __glibcxx_assert(__b != memory_order_acq_rel);

@@ -797,8 +815,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                              memory_order __m1,
                              memory_order __m2) noexcept
       {
-       memory_order __b2 = __m2 & __memory_order_mask;
-       memory_order __b1 = __m1 & __memory_order_mask;
+       memory_order __b2 __attribute__ ((__unused__))
+         = __m2 & __memory_order_mask;
+       memory_order __b1 __attribute__ ((__unused__))
+         = __m1 & __memory_order_mask;
        __glibcxx_assert(__b2 != memory_order_release);
        __glibcxx_assert(__b2 != memory_order_acq_rel);
        __glibcxx_assert(__b2 <= __b1);
@@ -812,8 +832,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                              memory_order __m1,
                              memory_order __m2) volatile noexcept
       {
-       memory_order __b2 = __m2 & __memory_order_mask;
-       memory_order __b1 = __m1 & __memory_order_mask;
+       memory_order __b2 __attribute__ ((__unused__))
+         = __m2 & __memory_order_mask;
+       memory_order __b1 __attribute__ ((__unused__))
+         = __m1 & __memory_order_mask;

        __glibcxx_assert(__b2 != memory_order_release);
        __glibcxx_assert(__b2 != memory_order_acq_rel);
diff --git a/libstdc++-v3/include/ext/new_allocator.h 
b/libstdc++-v3/include/ext/new_allocator.h
index 131718b8b2f..2e21a98409f 100644
--- a/libstdc++-v3/include/ext/new_allocator.h
+++ b/libstdc++-v3/include/ext/new_allocator.h
@@ -117,7 +117,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION

       // __p is not permitted to be a null pointer.
       void
-      deallocate(_Tp* __p, size_type __t)
+      deallocate(_Tp* __p, size_type __t __attribute__ ((__unused__)))
       {
 #if __cpp_aligned_new
        if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)


Changelog entry:

    libstdc++-v3/ChangeLog:

            * include/bits/atomic_base.h: Fix -Wunused-variable
            in system headers.
            * include/ext/new_allocator.h: Fix -Wunused-parameter
            in system headers.


Test suite confirmation:
All tests pass. Tested on both Manjaro and some Ubuntu 18.04 with gcc10
and gcc8 respectively.

On Mon, 24 Aug 2020 at 13:59, Jonathan Wakely <jwak...@redhat.com> wrote:

On 24/08/20 13:26 +0200, Krystian Kuźniarek via Libstdc++ wrote:
>Hi,
>
>First of all, sorry, I must have sent it as quoted-printable so spaces
and
>tabs are preserved.
>
>A description of the problem/bug and how your patch addresses it:
>I've got a small patch for -Wunused-variable and -Wunused-parameter in
>system headers. These are needed either for:
>1) __glibcxx_assert() that quickly goes away during an early compilation
>step leaving the warning
>2) code that conditionally is removed by preprocessor leaving the warning
>
>Testcases:
>N/A, it's only a warning.
>
>ChangeLog:
>Sorry, contrib/mklog.py didn't quite work for me.
>For some reason after instruction in line 129: "diff = PatchSet(data)" my
>"diff" variable is always empty.
>
>Bootstrapping and testing:
>Tested that manually by recompling GCC, unfolding all headers with
>`#include <stdc++.h>` and compiling what's been included by it.
>
>The patch itself:
>
>diff --git a/libstdc++-v3/include/bits/atomic_base.h
>b/libstdc++-v3/include/bits/atomic_base.h
>index 015acef83c4..1fbb2d78d28 100644
>--- a/libstdc++-v3/include/bits/atomic_base.h
>+++ b/libstdc++-v3/include/bits/atomic_base.h
>@@ -231,7 +231,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     _GLIBCXX_ALWAYS_INLINE void
>     clear(memory_order __m = memory_order_seq_cst) noexcept
>     {
>-      memory_order __b = __m & __memory_order_mask;
>+      memory_order __b __attribute__ ((__unused__)) =

Thanks for the patch.

Please put the line break before the '=' character i.e.

       memory_order __b __attribute__ ((__unused__))
         = __m & __memory_order_mask;

Other than that, the patch looks ok, and simple enough to not need a
copyright assignment for GCC.

A changelog entry and confirmation the testsuite still runs would be
preferred though.

Re: [PATCH] libstdc++: mark variables as possibly unused

Reply via email to