[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lmi] Stifling "unused" warnings
From: |
Vadim Zeitlin |
Subject: |
Re: [lmi] Stifling "unused" warnings |
Date: |
Fri, 29 Oct 2021 00:17:56 +0200 |
On Thu, 28 Oct 2021 16:16:05 +0000 Greg Chicares <gchicares@sbcglobal.net>
wrote:
GC> This particular unit test attempts to distill this situation:
GC>
GC> void mete0()
GC> {
GC> double volatile x;
GC> for(int j = 0; j < 100000; ++j)
GC> {
GC> x = i_upper_12_over_12_from_i_naive<double>()(0.04);
GC> x = i_from_i_upper_12_over_12_naive<double>()(0.04);
GC> x = d_upper_12_from_i_naive <double>()(0.04);
GC> x = net_i_from_gross_naive<double,365>()(0.04, 0.007, 0.003);
GC> }
GC> stifle_unused_warning(x);
GC> }
GC>
GC> where we had moved the "stifle_unused_warning(x);" line from its
GC> original location (right below the "double volatile x;" line)
GC> because, as the commit message explains:
GC>
GC> Otherwise this attempt to suppress gcc -Wunused-but-set-variable warning
GC> results in -Wuninitialized-const-reference from clang because the
GC> variable was indeed not initialized yet when it was referenced from
GC> inside stifle_warning_for_unused_value().
GC>
GC> Now I think I see my mistake. In reality, the problem is simple--the
GC> reason why the original code failed seems obvious if we isolate it:
GC>
GC> double volatile x;
GC> stifle_warning_for_unused_value(x);
GC>
GC> I.e., an uninitialized variable has no unused value because it has no value.
GC> Somehow I had thought that the problem was that the first 399,999 values
GC> assigned to 'x' were considered "used", but the 400,000th was "unused";
GC> but actually the value of 'x' is never referenced, and 'x' is a write-only
GC> sink.
Yes, exactly.
GC> > I don't fully understand what's going on here, but I just can't get any
GC> > -Wunused-variable for "d" with clang 12 unless I remove the assignment to
GC> > it inside the loop, even removing volatile doesn't help. I.e. somehow
GC> > initializing a variable and not using it provokes this warning, but
GC> > initializing a variable and then assigning to it suppresses it.
GC>
GC> Thus, as an alternative to commit 0380e928ce:
GC>
GC> double volatile x;
GC> - stifle_warning_for_unused_value(x);
GC> for(int j = 0; j < 100000; ++j)
GC> ...
GC> + stifle_warning_for_unused_value(x);
GC>
GC> this would probably have sufficed:
GC>
GC> - double volatile x;
GC> + double volatile x {};
GC> stifle_warning_for_unused_value(x);
Yes, it would, but while initializing variables is in general a good idea,
it seems a tiny bit strange to do it only to avoid a warning which itself
occurs only due to a workaround for another warning (i.e. it's not even a
naturally occurring warning, but a second order effect).
GC> The original suggestion here:
GC> https://herbsutter.com/2009/10/18/mailbag-shutting-up-compiler-warnings/
GC> was apparently
GC> template<class T> void ignore(T&) { }
GC> but was later changed to
GC> template<class T> void ignore(T const&) { }
GC> because
GC> | The explicit const lets you also handle temporaries.
GC> | I’ll update the post to add the const.
GC>
GC> That's an interesting use case, so I added tests for it in a commit
GC> that I just pushed. Although interesting, I'm not convinced that
GC> it's actually useful.
Me neither, which is why I asked. IMO you should take "T&" in this
function if you want to actively disallow passing temporaries to it.
Otherwise it should take either "T const&", if you accept to have to
initialize all the variables used with it, or "T&&" if you don't want to
bother with it.
GC> Assuming this unit test now covers every use case actually in lmi,
GC> as well as the ignore-a-temporary idea above, we can discuss
GC> implementations. Here you're set up to use gcc++-11 and clang,
GC> whereas I'm not, so it might be more efficient to ask you to
GC> try each implementation and reply with your findings.
The latest stifle_unused_warning() version works fine with both gcc and
clang and the tests seem representative to me, so AFAICS this problem is
completely solved now.
GC> The candidate implementations we've discussed include:
GC> (1) template<typename T> constexpr void stifle_unused_warning(T const&) {}
GC> (2) template<typename T> constexpr void stifle_unused_warning(T&) {}
GC> (3) template<typename T> constexpr void stifle_unused_warning(T&&) {}
GC> I also looked at boost's:
GC>
https://github.com/boostorg/core/blob/12f5f51427fbc9d27f56e5de002b0008fea420c9/include/boost/core/ignore_unused.hpp
GC> which seems curious.
[...]
Boost tries to support even more C++ compilers than wxWidgets does. I
don't envy them their task, and I think this can be valuable, but you can
definitely do better if you only support (reasonably) standard-compliant
C++20 compilers as we do.
GC> I don't think it's necessary to accommodate multiple arguments.
Yes, I really don't know why did they do it. I tried to think about some
scenarios involving eldritch horrors (a.k.a. C preprocessor macros), but
even then I couldn't come up with a realistic example where this would be
needed.
GC> I'd be inclined to try a single (not overloaded) implementation
GC> using a "universal reference", i.e.:
GC>
GC> template<typename T> constexpr void stifle_unused_warning(T&&) {}
GC>
GC> and I'm so hopeful it'll address all use cases that I've gone ahead
GC> and committed it--pushed just now. What do you think?
I think it's perfectly fine if you want to support passing temporaries to
this function (and I don't see any reason to forbid this).
GC> Does the unit test encompass all of lmi's use cases as well as any
GC> others that we or other people have thought of? And does the
GC> universal-reference implementation address all of this cases in the
GC> best way?
AFAICS, yes.
GC> And can the 'd' test be simplified to
GC> int volatile d;
GC> stifle_unused_warning(d);
GC> in light of the discussion above? (If so, then it becomes a trivial
GC> echo of the 'a' case.)
Yes, I think it's the same test written in a more complicated way,
basically.
Thanks for fixing this!
VZ
pgpFMfVouHClZ.pgp
Description: PGP signature
- [lmi] Stifling "unused" warnings, Greg Chicares, 2021/10/26
- Re: [lmi] Stifling "unused" warnings, Vadim Zeitlin, 2021/10/26
- Message not available
- Re: [lmi] Stifling "unused" warnings, Greg Chicares, 2021/10/26
- Re: [lmi] Stifling "unused" warnings, Vadim Zeitlin, 2021/10/27
- Message not available
- Re: [lmi] Stifling "unused" warnings, Greg Chicares, 2021/10/27
- Re: [lmi] Stifling "unused" warnings, Vadim Zeitlin, 2021/10/27
- Message not available
- Re: [lmi] Stifling "unused" warnings, Greg Chicares, 2021/10/27
- Re: [lmi] Stifling "unused" warnings, Vadim Zeitlin, 2021/10/27
- Message not available
- Re: [lmi] Stifling "unused" warnings, Greg Chicares, 2021/10/28
- Re: [lmi] Stifling "unused" warnings,
Vadim Zeitlin <=