Re: [lmi] Is '-Wno-unused-variable' still needed?

[Vadim Zeitlin]

On Tue, 26 Oct 2021 13:59:38 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 10/24/21 9:37 PM, Vadim Zeitlin wrote:
GC> [...]
GC> > ---------------------------------- >8 
--------------------------------------
GC> > ledger_evaluator.cpp:690:39: error: unused variable 'f4' 
[-Werror,-Wunused-variable]
GC> >     std::pair<int,oenum_format_style> f4(2, oe_format_percentage);
GC> >                                       ^
GC> > ledger_evaluator.cpp:688:39: error: unused variable 'f2' 
[-Werror,-Wunused-variable]
GC> >     std::pair<int,oenum_format_style> f2(2, oe_format_normal);
GC> >                                       ^
GC> > 2 errors generated.
GC> > ---------------------------------- >8 
--------------------------------------
GC> [...]
GC> >  I'm not sure if I should move this LMI_CXX_ADD_IF_SUPPORTED() to
GC> > clang-only section of configure or if we could fix it, even for clang, and
GC> 
GC> Done.
GC> 
GC> > then just delete this line.
GC> Done.

 Thanks!

 Unfortunately, this is still not quite the end of the story, as
re-enabling this warning for clang uncovered a few more occurrences of it,
all in the same test:

---------------------------------- >8 --------------------------------------
currency_test.cpp:93:24: error: unused variable 'zero' 
[-Werror,-Wunused-variable]
    constexpr currency zero {};
                       ^
currency_test.cpp:168:10: error: unused variable 'compile_time_constant_pos' 
[-Werror,-Wunused-variable]
    auto compile_time_constant_pos( 9007199254740992_cents);
         ^
currency_test.cpp:169:10: error: unused variable 'compile_time_constant_neg' 
[-Werror,-Wunused-variable]
    auto compile_time_constant_neg(-9007199254740992_cents);
         ^
currency_test.cpp:295:27: error: unused variable 'z' [-Werror,-Wunused-variable]
        currency volatile z = std::min(extreme, value);
                          ^
currency_test.cpp:306:27: error: unused variable 'z' [-Werror,-Wunused-variable]
        currency volatile z = std::min(extreme, value);
                          ^
---------------------------------- >8 --------------------------------------

 I think the first one might actually point to a typo in the test and
instead of

    constexpr currency zero {};
    LMI_TEST(   0 == a0.m_);

you might have meant to write

    constexpr currency zero {};
    LMI_TEST(   0 == zero.m_);

(a0 is already tested just above).

 But I think all the other ones will have to be suppressed using
stifle_warning_for_unused_variable(), the only alternative I see is
disabling the warning globally for this file using a clang-specific pragma,
but it doesn't really seem better.

 So I'd like to propose the following patch which I've tested to work with
both clang 12 and gcc 11:

---------------------------------- >8 --------------------------------------
diff --git a/currency_test.cpp b/currency_test.cpp
index e01901bab..ccd4fddb9 100644
--- a/currency_test.cpp
+++ b/currency_test.cpp
@@ -91,7 +91,7 @@ void currency_test::test_default_ctor()
     LMI_TEST(0.00 == a0.d());
     LMI_TEST(   0 == a0.m_);
     constexpr currency zero {};
-    LMI_TEST(   0 == a0.m_);
+    LMI_TEST(   0 == zero.m_);
 }

 void currency_test::test_copy_ctor()
@@ -166,7 +166,11 @@ void currency_test::test_literals()
     // These are evaluated at compile time, even though this is not
     // a constexpr context:
     auto compile_time_constant_pos( 9007199254740992_cents);
+    stifle_warning_for_unused_variable(compile_time_constant_pos);
+
     auto compile_time_constant_neg(-9007199254740992_cents);
+    stifle_warning_for_unused_variable(compile_time_constant_neg);
+
     // These would be compile-time errors:
 //  auto error_at_compile_time_pos( 9007199254740993_cents);
 //  auto error_at_compile_time_neg(-9007199254740993_cents);
@@ -293,6 +297,7 @@ void currency_test::mete_humongous()
     for(int i = 0; i < 100000; ++i)
         {
         currency volatile z = std::min(extreme, value);
+        stifle_warning_for_unused_variable(z);
         }
 }

@@ -304,6 +309,7 @@ void currency_test::mete_infinite()
     for(int i = 0; i < 100000; ++i)
         {
         currency volatile z = std::min(extreme, value);
+        stifle_warning_for_unused_variable(z);
         }
 }

---------------------------------- >8 --------------------------------------

 I've also pushed this commit as bcb72b820 to fix-clang-unused branch on
GitHub, so you can fetch it from there and cherry-pick or even
fast-forward, if you agree with the changes made in it.

 Thanks in advance,
VZ

pgpHCwOvUc3s3.pgp
Description: PGP signature