[libc] build with -Werror

[nickdesaulniers]

A recent commit introduced warnings observable when building unit tests. If the
unit tests don't fail when warnings are introduced into the build, then we
might fail to notice them in the stream of output from check-libc.

Link: https://github.com/llvm/llvm-project/pull/72763/files#r1410932348

[llvmbot]

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

A recent commit introduced warnings observable when building unit tests. If the
unit tests don't fail when warnings are introduced into the build, then we
might fail to notice them in the stream of output from check-libc.

Link: https://github.com/llvm/llvm-project/pull/72763/files#r1410932348

---

Full diff: https://github.com/llvm/llvm-project/pull/73966.diff

1 Files Affected:

• (modified) libc/cmake/modules/LLVMLibCObjectRules.cmake (+1)

diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
index d5df27a2dcb4341..da14d4767bff2c6 100644
--- a/libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -35,6 +35,7 @@ function(_get_common_compile_options output_var flags)
     list(APPEND compile_options "-fno-rtti")
     list(APPEND compile_options "-Wall")
     list(APPEND compile_options "-Wextra")
+    list(APPEND compile_options "-Werror")
     list(APPEND compile_options "-Wconversion")
     list(APPEND compile_options "-Wno-sign-conversion")
     list(APPEND compile_options "-Wimplicit-fallthrough")

[jhuber6]

Not permitting any warnings is a pretty big change, likely we'll have some failing builds until they're cleaned up.

[nickdesaulniers]

Is there a way to rekick presubmit tests? I thought if I forced pushed it would, but now it doesn't seem to be happening. Either I misremember or that's not wired up for llvmlibc like the rest of llvm.

Not permitting any warnings is a pretty big change, likely we'll have some failing builds until they're cleaned up.

Besides what coverage we have in presubmits, do you know where else I should look for breakages? Are the build bots all considered post submit (and not pre submit). Sorry, I'm new here.

[jhuber6]

Is there a way to rekick presubmit tests? I thought if I forced pushed it would, but now it doesn't seem to be happening. Either I misremember or that's not wired up for llvmlibc like the rest of llvm.

Not permitting any warnings is a pretty big change, likely we'll have some failing builds until they're cleaned up.

Besides what coverage we have in presubmits, do you know where else I should look for breakages? Are the build bots all considered post submit (and not pre submit). Sorry, I'm new here.

Unsure, I think there's some libc tests that run precommit runs but definitely not all of them. I've seen a handful of warnings before so I don't know if they've all been resolved here.

[jhuber6]

If you don't want to turn it on globally, but have some specific ones in mind, you can probably use -Werror=. For example, I'd like -Werror=global-constructors because they really shouldn't appear by accident.

[nickdesaulniers]

I think it will be good for the long term health of the project that we can compile llvmlibc out of the box without warnings. That doesn't preclude the use of -Wno-* to disable warnings that aren't helpful/applicable, but we really should be striving to ensure warnings aren't creeping in.

[lntue]

How about we add a cmake control flag for this so that other users can control it however fit them, and keep it on by default on our build scripts and bots?

[gchatelet]

Is there a way to rekick presubmit tests? I thought if I forced pushed it would, but now it doesn't seem to be happening. Either I misremember or that's not wired up for llvmlibc like the rest of llvm.

I don't know how to re-kick them but I ran into the same failure yesterday. I rebased + forced push and it buildkite kicked in ¯\_(ツ)_/¯

[nickdesaulniers]

How about we add a cmake control flag for this so that other users can control it however fit them, and keep it on by default on our build scripts and bots?

done PTAL

[SchrodingerZhu]

There is a retry button over there:

But, it requires access permission.

[jhuber6]

I wonder if we should mention that libc expects to compile with no warnings in the documentation somewhere. https://libc.llvm.org/dev/index.html.

[nickdesaulniers]

I wonder if we should mention that libc expects to compile with no warnings in the documentation somewhere. https://libc.llvm.org/dev/index.html.

Done in a224d0b, PTAL

[nickdesaulniers]

Now that the build bots are explicitly red, I'm happy to revert this as we get things fixed up.

https://lab.llvm.org/buildbot/#/builders?tags=libc

[nickdesaulniers]

32b arm failures: https://lab.llvm.org/buildbot/#/builders/229/builds/20596

aarch64: https://lab.llvm.org/buildbot/#/builders/223/builds/32131

x86: https://lab.llvm.org/buildbot/#/builders/163/builds/48002

x86 gcc: https://lab.llvm.org/buildbot/#/builders/250/builds/14891

[jhuber6]

The GPU build is fine FYI.