Repository: protocolbuffers/protobuf (language: python)

Issue:
Build with -Wundef
This allows downstream consumers of protobuf to enable Wundef in their code which consumes protobuf APIs.


Discussion / hints:
Are there further changes needed in order to get this in?
Yes, the bzl changes aren't actually doing anything
@cramertj Are you still interested in this change? Per @mkruskal-google's comment in https://github.com/protocolbuffers/protobuf/pull/17845#discussion_r1722592892, this will need to be done in https://github.com/protocolbuffers/protobuf/blob/main/ci/common.bazelrc instead.
@zhangskz I've pushed changes to ci/common.bazelrc, but I don't know how to test locally-- let me know if there is a process I should use here.
I kicked off tests, but you'll probably need to rebase as it's been a while
Looks like there are a lot of other test failures with -Wall -Wextra or the other warnings, so I've pulled it down to a more reduced set. It's difficult to know when it's working, however, without the ability to check locally.
I see from the test runs that we have failures with both `shadow` and `redundant-decls`, so we probably need to limit this to `undef` and `pointer-arith` for now.
@acozzette Updated!
@cramertj Thank you for the fix. Unfortunately I don't think we can easily enforce `-Wundef` on our build, because I found that one of our dependencies is not compatible with it: https://github.com/google/benchmark If you feel up to sending another pull request there to fix them first, then that would make this possible, though.

For now I plan to submit a change that at least makes our code compatible with `-Wundef`. There won't be anything enforcing that it stays that way, however.

Fix patch:
diff --git a/ci/Linux.bazelrc b/ci/Linux.bazelrc
index fb91d049b8ad..354a7582c31c 100644
--- a/ci/Linux.bazelrc
+++ b/ci/Linux.bazelrc
@@ -2,5 +2,6 @@ import common.bazelrc
 
 build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
 build --cxxopt="-Woverloaded-virtual"
+build --copt="-Wpointer-arith"
 build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations"
 
diff --git a/ci/macOS.bazelrc b/ci/macOS.bazelrc
index 847810e8a4a3..2fd5a2a3db22 100644
--- a/ci/macOS.bazelrc
+++ b/ci/macOS.bazelrc
@@ -2,6 +2,7 @@ import common.bazelrc
 
 build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
 build --cxxopt="-Woverloaded-virtual"
+build --copt="-Wpointer-arith"
 build --copt="-Werror" --copt="-Wno-sign-compare" --copt="-Wno-sign-conversion" --copt="-Wno-error=sign-conversion" --copt="-Wno-deprecated-declarations"
 common --repo_env=BAZEL_NO_APPLE_CPP_TOOLCHAIN=1
 common --xcode_version_config=@com_google_protobuf//.github:host_xcodes
diff --git a/src/google/protobuf/message_lite.h b/src/google/protobuf/message_lite.h
index 96a30911054b..fa83a979bf82 100644
--- a/src/google/protobuf/message_lite.h
+++ b/src/google/protobuf/message_lite.h
@@ -178,7 +178,7 @@ class PROTOBUF_EXPORT CachedSize {
   // NOLINTNEXTLINE(google-explicit-constructor)
   constexpr CachedSize(Scalar desired) noexcept : atom_(desired) {}
 
-#if PROTOBUF_BUILTIN_ATOMIC
+#ifdef PROTOBUF_BUILTIN_ATOMIC
   constexpr CachedSize(const CachedSize& other) = default;
 
   Scalar Get() const noexcept {
@@ -233,7 +233,7 @@ class PROTOBUF_EXPORT CachedSize {
 #endif
 
  private:
-#if PROTOBUF_BUILTIN_ATOMIC
+#ifdef PROTOBUF_BUILTIN_ATOMIC
   mutable Scalar atom_;
 #else
   mutable std::atomic<Scalar> atom_;
diff --git a/upb/message/internal/message.c b/upb/message/internal/message.c
index fea8822bc91a..1ba26df582db 100644
--- a/upb/message/internal/message.c
+++ b/upb/message/internal/message.c
@@ -85,7 +85,7 @@ bool UPB_PRIVATE(_upb_Message_EnsureAvailable)(struct upb_Message* msg,
   return true;
 }
 
-#if UPB_TRACING_ENABLED
+#ifdef UPB_TRACING_ENABLED
 static void (*_message_trace_handler)(const upb_MiniTable*, const upb_Arena*);
 
 void upb_Message_LogNewMessage(const upb_MiniTable* m, const upb_Arena* arena) {
diff --git a/upb/mini_descriptor/decode.c b/upb/mini_descriptor/decode.c
index 1094fc0435b2..c916f98c5a85 100644
--- a/upb/mini_descriptor/decode.c
+++ b/upb/mini_descriptor/decode.c
@@ -742,7 +742,7 @@ static upb_MiniTable* upb_MtDecoder_DoBuildMiniTableWithBuf(
   decoder->table->UPB_PRIVATE(dense_below) = 0;
   decoder->table->UPB_PRIVATE(table_mask) = -1;
   decoder->table->UPB_PRIVATE(required_count) = 0;
-#if UPB_TRACING_ENABLED
+#ifdef UPB_TRACING_ENABLED
   // MiniTables built from MiniDescriptors will not be able to vend the message
   // name unless it is explicitly set with upb_MiniTable_SetFullName().
   decoder->table->UPB_PRIVATE(full_name) = 0;
diff --git a/upb/reflection/descriptor_bootstrap.h b/upb/reflection/descriptor_bootstrap.h
index c0b760cfa1fb..0a7961d2afa0 100644
--- a/upb/reflection/descriptor_bootstrap.h
+++ b/upb/reflection/descriptor_bootstrap.h
@@ -6,7 +6,7 @@
 #if defined(UPB_BOOTSTRAP_STAGE) && UPB_BOOTSTRAP_STAGE == 0
 // This header is checked in.
 #include "upb/reflection/stage0/google/protobuf/descriptor.upb.h"
-#elif UPB_BOOTSTRAP_STAGE == 1
+#elif defined(UPB_BOOTSTRAP_STAGE) && UPB_BOOTSTRAP_STAGE == 1
 // This header is generated at build time by the bootstrapping process.
 #include "upb/reflection/stage1/google/protobuf/descriptor.upb.h"
 #else
diff --git a/upb_generator/plugin_bootstrap.h b/upb_generator/plugin_bootstrap.h
index 87afc06fbbd9..da9fe6875abd 100644
--- a/upb_generator/plugin_bootstrap.h
+++ b/upb_generator/plugin_bootstrap.h
@@ -6,7 +6,7 @@
 #if defined(UPB_BOOTSTRAP_STAGE) && UPB_BOOTSTRAP_STAGE == 0
 // This header is checked in.
 #include "upb_generator/stage0/google/protobuf/compiler/plugin.upb.h"
-#elif UPB_BOOTSTRAP_STAGE == 1
+#elif defined(UPB_BOOTSTRAP_STAGE) && UPB_BOOTSTRAP_STAGE == 1
 // This header is generated at build time by the bootstrapping process.
 #include "upb_generator/stage1/google/protobuf/compiler/plugin.upb.h"
 #else


Test patch:
diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc
index 1778672134cb..f05b62af8979 100644
--- a/src/google/protobuf/compiler/command_line_interface_unittest.cc
+++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc
@@ -84,7 +84,7 @@ using google::protobuf::io::win32::write;
 
 // Disable the whole test when we use tcmalloc for "draconian" heap checks, in
 // which case tcmalloc will print warnings that fail the plugin tests.
-#if !GOOGLE_PROTOBUF_HEAP_CHECK_DRACONIAN
+#if !defined(GOOGLE_PROTOBUF_HEAP_CHECK_DRACONIAN)
 
 
 namespace {