[RFC] Warning suppression policy

[flang-rt][build] Disable two build-time warning of '-Wuninitialized' from g++ compiler by zjlcd · Pull Request #174918 · llvm/llvm-project · GitHub (and some other related PRs) try to silence a false gcc warning by adding pragmas around the function where gcc reports an unitialized variable:

+#if defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wuninitialized"
+#endif

static Type Value() {
  ...
}

+if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif

where an alternative is to just zero-initialize (instead-of default-initialize) the variable:

     constexpr std::size_t alignment =
         std::max(alignof(Type), alignof(std::uint64_t));
-    alignas(alignment) char data[sizeof(Type)];
+    alignas(alignment) char data[sizeof(Type)]{};

I think the former is the more probablematic trade-off:

  1. Many more lines
  2. Applies to a region of code, not to the specific occurance of the warning
  3. Does not tell why the pragma is there (a comment could solve that)
  4. Potentially also silences other, legitimate warnings
  5. Also silences Clang warnings, even though it does not report this false positive (see Compiler Explorer). This could be solved with another #if defined(__clang__) condition as done elsewhere.
  6. The additional zero-initialization is removed by any compiler with even the lowest level of optimizations enabled (see Compiler Explorer).

My question is, should we still introduce such pragmas?


Not related to the question, but to the PR: I think the gcc warning shows a different issue, namely using reinterpret_cast to access memory using two different non-char types (here: std::uint64_t and __float128) is undefined behavior, since the compiler can assume that the two accesses do not alias:

This rule enables type-based alias analysis, in which a compiler assumes that the value read through a glvalue of one type is not modified by a write to a glvalue of a different type

I’d say zero initialize.

When/How would we know the issue with GNU has been addressed and the WARs can be removed without the bogus message returning?

Do we know why pragmas were used in the first place? Was there a concern that the zero-initialization would not be optimized away?

In general I rather disable GCC warnings in CMake when they prove to have false positives:

  1. this preserves clang warnings, which is the most important for the project.
  2. this can be done on specific GCC versions so that when GCC improves we can enable the warning again.

Here is how it looks in practice: llvm-project/mlir/CMakeLists.txt at main · llvm/llvm-project · GitHub

To be fair, this is a zero-initialization of 120 bytes