Harmless unused variable warnings on clang with -O -Wall

Issue #468 resolved
Dan Bonachea created an issue

Building any UPC++ program against a library backed by clang and passing the -O -Wall command-line options to upcxx results in warnings such as the following:

$ upcxx -O -Wall hello_upcxx.cpp 
...
<redacted>/upcxx.opt.gasnet_seq.smp/include/upcxx/device_allocator.hpp:170:30: warning: 
      unused variable 'hs' [-Wunused-variable]
        backend::heap_state *hs =
                             ^
<redacted>/upcxx.opt.gasnet_seq.smp/include/upcxx/device_allocator.hpp:185:28: warning: 
      unused variable 'hs' [-Wunused-variable]
      backend::heap_state *hs =
                           ^
2 warnings generated.

The warnings are harmless but annoying, and appear to affect all supported versions of LLVM clang and most (all?) supported versions of Apple XCode clang.

This defect first appeared in UPC++ 2020.11.0 and persists in 2021.3.0.

Recommended workaround is to append -Wno-unused-variable on the command line.

First reported by @Scott Baden

Comments (3)

  1. Dan Bonachea reporter

    Upon deeper investigation there are also similar caller-dependent -Wunused warnings elsewhere in the public headers:

    • rput.hpp on code invoking upcxx::rput()
    • diagnostic.hpp on code invoking upcxx_memberof()

    It's worth noting that upcxx/upcxx-meta already automatically inject -Wno-unused earlier on the CXXFLAGS part of the compiler command line for most/all compilers, including this one.

    Part of the problem here is clang's parsing algorithm for warning options. GCC is smart enough to parse-Wno-unused -Wall as meaning "all of Wall except for unused warnings", whereas clang takes the -Wall macro option as overwriting the previous -Wno-unused option. This latter behavior causes the user-provided -Wall to overwrite the library's earlier -Wno-unused, which leads any unused warnings in the public headers to be "exposed", despite the fact the user did not specifically request unused warnings (and almost certainly doesn't care about instances in our headers).

    For the record I have a very low opinion in general of the quality/utility of -Wunused warnings, especially when it comes to well-maintained library code; where unused variables/expressions/whatever are most often the expected result of conditional compilation, and basically always guaranteed to be harmless and have no performance impact (ie the optimizer is essentially spamming about the fact it's performing dead code elimination, which is a normal/expected part of its operation).

    That being said, I recognize some users might find it valuable to use -Wunused on their own code, so there is some value to ensuring our public headers don't interfere with that desire. GASNet public headers handle this (in part) via scoped warning suppression pragmas, which are a bit cumbersome to deploy but we might eventually go there. In the meantime, the current known "offenders" in the public headers were few in number, so pull request #346 addresses them point-wise.

  2. Log in to comment