[PATCH] D123481: Do not build with Werror by default (Bazel build)

[Geoffrey Martin-Noble via Phabricator via llvm-commits]

GMNGeoffrey added a comment.

In D123481#3442718 <https://reviews.llvm.org/D123481#3442718>, @kuhar wrote:

> +1, I had to patch this locally in my project for exactly the reasons you mentioned

I don't understand what you mean about having to patch it locally in a different project. bazelrc files are only applicable to the project itself, not anything that uses it as a dependency.



================
Comment at: utils/bazel/.bazelrc:38
+# Use `-Wall` for Clang.
+build:generic_clang --copt=-Wall --host_copt=-Wall
 
----------------
[Here for threading]

I'm pretty strongly against warnings in default build configurations. In my experience, it inevitably leads to an ever-growing pile of warning spam. If we turn it off here, I'm going to turn it on explicitly on the CI. The issue with that is that then users have things that only fail on the CI (Bazel will hide warnings from cached actions, irritatingly). I take your point about users on different platforms and compiler versions. Notably they can override bazelrc options themselves and pass `--copt=-Wno-error --host_copt=-Wno-error`. Have you actually run into issues or is this theoretical right now? The better workflow IMO would be for someone to switch to building locally with these options and send a patch to fix things for the new compiler/platform. Opting in to ignoring warnings seems better to me, but it depends on the frequency of this issue.


================
Comment at: utils/bazel/.bazelrc:74
 
-# Use `-Werror` for GCC to make sure warnings don't slip past.
-build:generic_gcc --copt=-Werror --host_copt=-Werror
----------------
Why would things be different for gcc than for clang?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123481/new/

https://reviews.llvm.org/D123481