Backport windows fixes with INFINITY/NAN (#19389)

mkruskal-google · haberman · protobuf-github-bot · web-flow · commit acaece69cfe7 · 2024-11-25T13:37:51.000-08:00
* Fixed `NAN`/`INFINITY` definitions to work on Windows 11 SDK. Fixes: #17308, #17238 Confirmation that this addresses the issue: grpc/grpc#37210 (comment) PiperOrigin-RevId: 652541857 * Fix compilation in Clang C23 mode. In C23, `false` is now of type `bool`, not `int`. This turned a couple places where `false` was returned instead of `NULL` into errors. The workaround in upb for Windows's broken NAN macro causes an error in Clang under C23, `cannot compile this static initializer yet`, I believe because `0.0 / 0.0` is not valid in constant-evaluation. This seems like it's probably a legitimate result of C23 standardizing constexpr, although it _could_ be a clang bug. In any case, refine the workaround a bit, to avoid this problem. I've also reverted the kUpb_FltInfinity/kUpb_Infinity back to their former definitions, as INFINITY wasn't broken by the windows header, only NAN. PiperOrigin-RevId: 683152452 * Fix const INFINITY issue on MSVC. We previously hit this for NAN, but it appears the latest version of MSVC used in windows github runners has the same issue for INFINITY. The same strategy can be applied here. This will be backported to fix broken release branches. PiperOrigin-RevId: 699390594 * Regenerate stale files * Update GitHub Actions to use macOS-13 ahead of macOS-12 deprecation. See actions/runner-images#10721. PiperOrigin-RevId: 684546079 * Pin to python 3.12 in upb/macos tests. Python 3.13 seems to have some issues with our dependencies and both numpy and setuptools fails to install. PiperOrigin-RevId: 689955916 * Fix bad cherry-pick --------- Co-authored-by: Joshua Haberman <haberman@google.com> Co-authored-by: Protobuf Team Bot <protobuf-github-bot@google.com>
diff --git a/.github/BUILD.bazel b/.github/BUILD.bazel
@@ -1,5 +1,5 @@
 # This information is extracted from the MacOS runner specs located at:
-# https://github.com/actions/runner-images/blob/main/images/macos/macos-12-Readme.md
+# https://github.com/actions/runner-images/blob/main/images/macos/macos-13-Readme.md
 #
 # When updating, also ensure the "xcode_destination" entries in
 # `.github/workflows/test_objectivec.yml` are supported for the given versions
diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml
@@ -321,17 +321,17 @@ jobs:
       matrix:
         include:
           - name: MacOS Bazel
-            os: macos-12
-            cache_key: macos-12
+            os: macos-13
+            cache_key: macos-13
             bazel: test //src/... //third_party/utf8_range/...
           - name: MacOS Bazel 7
-            os: macos-12
-            cache_key: macos-12-bazel7
+            os: macos-13
+            cache_key: macos-13-bazel7
             bazel: test //src/... //third_party/utf8_range/...
             bazel_version: '7.1.2'
           - name: MacOS Apple Silicon (build only) Bazel
-            os: macos-12
-            cache_key: macos-12-arm
+            os: macos-13
+            cache_key: macos-13-arm
             # Current github runners are all Intel based, so just build/compile
             # for Apple Silicon to detect issues there.
             bazel: build --cpu=darwin_arm64 //src/... //third_party/utf8_range/...
@@ -366,7 +366,7 @@ jobs:
         include:
           # TODO: investigate and fix
           # - name: MacOS CMake
-          #   os: macos-12
+          #   os: macos-13
           #   flags: -DCMAKE_CXX_STANDARD=14
           #   cache-prefix: macos-cmake
           - name: Windows CMake
diff --git a/.github/workflows/test_objectivec.yml b/.github/workflows/test_objectivec.yml
@@ -25,11 +25,11 @@ jobs:
           destination: "platform=macOS"
           xc_project: "ProtocolBuffers_OSX.xcodeproj"
         - platform: "iOS"
-          destination: "platform=iOS Simulator,name=iPhone 13,OS=latest"
+          destination: "platform=iOS Simulator,name=iPhone 14,OS=latest"
           xc_project: "ProtocolBuffers_iOS.xcodeproj"
 
     name: Xcode ${{ matrix.platform}} ${{ matrix.xc_config }}
-    runs-on: macos-12
+    runs-on: macos-13
     env:
       DEVELOPER_DIR: /Applications/Xcode_14.1.app/Contents/Developer
     steps:
@@ -72,7 +72,7 @@ jobs:
         PLATFORM: ["ios", "macos", "tvos", "watchos"]
         CONFIGURATION: ["Debug", "Release"]
         include:
-          - OS: macos-12
+          - OS: macos-13
             XCODE: "14.1"
     name: CocoaPods ${{ matrix.PLATFORM }} ${{ matrix.CONFIGURATION }}
     runs-on: ${{ matrix.OS }}
@@ -120,7 +120,7 @@ jobs:
         - platform: "macOS"
           bazel_targets: //objectivec/...
     name: Bazel ${{ matrix.platform }} ${{ matrix.config.name }}
-    runs-on: macos-12
+    runs-on: macos-13
     steps:
       - name: Checkout pending changes
         uses: protocolbuffers/protobuf-ci/checkout@v3
diff --git a/.github/workflows/test_php.yml b/.github/workflows/test_php.yml
@@ -158,7 +158,7 @@ jobs:
         version: ['8.2', '8.3']
 
     name: MacOS PHP ${{ matrix.version }}
-    runs-on: macos-12
+    runs-on: macos-13
     steps:
       - name: Checkout pending changes
         uses: protocolbuffers/protobuf-ci/checkout@v3
diff --git a/.github/workflows/test_python.yml b/.github/workflows/test_python.yml
@@ -65,7 +65,7 @@ jobs:
             flags: --define=use_fast_cpp_protos=true
 
     name: MacOS ${{ matrix.type }} ${{ matrix.version }}
-    runs-on: macos-12
+    runs-on: macos-13
     steps:
       - name: Checkout pending changes
         uses: protocolbuffers/protobuf-ci/checkout@v3
diff --git a/.github/workflows/test_ruby.yml b/.github/workflows/test_ruby.yml
@@ -128,7 +128,7 @@ jobs:
         - { version: "3.3", ffi: FFI }
 
     name: MacOS Ruby ${{ matrix.version }}${{ matrix.ffi == 'FFI' && ' FFI' || '' }}
-    runs-on: macos-12
+    runs-on: macos-13
     steps:
       - name: Checkout pending changes
         uses: protocolbuffers/protobuf-ci/checkout@v3
diff --git a/.github/workflows/test_upb.yml b/.github/workflows/test_upb.yml
@@ -95,14 +95,15 @@ jobs:
           - { name: "macOS", bazel-command: "test" }
           - { name: "macOS ARM (build only)", bazel-command: "build", flags: "--cpu=darwin_arm64" }
     name: ${{ matrix.config.name }}
-    runs-on: macos-12
+    runs-on: macos-13
     steps:
       - name: Checkout pending changes
         uses: protocolbuffers/protobuf-ci/checkout@v3
         with:
           ref: ${{ inputs.safe-checkout }}
       - uses: actions/setup-python@61a6322f88396a6271a6ee3565807d608ecaddd1 # v4.7.0
         with:
+          python-version: 3.12
           cache: pip
           cache-dependency-path: 'python/requirements.txt'
       - name: Run tests
@@ -175,11 +176,11 @@ jobs:
           # supported Python versions and assume this gives us sufficient test
           # coverage.
           - { os: ubuntu-latest, python-version: "3.8", architecture: x64, type: 'binary' }
-          - { os: macos-12, python-version: "3.8", architecture: x64, type: 'binary' }
+          - { os: macos-13, python-version: "3.8", architecture: x64, type: 'binary' }
           - { os: ubuntu-latest, python-version: "3.12", architecture: x64, type: 'binary' }
           - { os: macos-13, python-version: "3.12", architecture: x64, type: 'binary' }
           - { os: ubuntu-latest, python-version: "3.8", architecture: x64, type: 'source' }
-          - { os: macos-12, python-version: "3.8", architecture: x64, type: 'source' }
+          - { os: macos-13, python-version: "3.8", architecture: x64, type: 'source' }
           - { os: ubuntu-latest, python-version: "3.12", architecture: x64, type: 'source' }
           - { os: macos-13, python-version: "3.12", architecture: x64, type: 'source' }
 
diff --git a/php/ext/google/protobuf/php-upb.c b/php/ext/google/protobuf/php-upb.c
@@ -6161,7 +6161,7 @@ upb_Array* upb_Array_DeepClone(const upb_Array* array, upb_CType value_type,
   for (size_t i = 0; i < size; ++i) {
     upb_MessageValue val = upb_Array_Get(array, i);
     if (!upb_Clone_MessageValue(&val, value_type, sub, arena)) {
-      return false;
+      return NULL;
     }
     upb_Array_Set(cloned_array, i, val);
   }
@@ -12204,9 +12204,35 @@ upb_Extension* UPB_PRIVATE(_upb_Message_GetOrCreateExtension)(
 
 // Must be last.
 
-const float kUpb_FltInfinity = INFINITY;
-const double kUpb_Infinity = INFINITY;
-const double kUpb_NaN = NAN;
+// The latest win32 SDKs have an invalid definition of NAN.
+// https://developercommunity.visualstudio.com/t/NAN-is-no-longer-compile-time-constant-i/10688907
+//
+// Unfortunately, the `0.0 / 0.0` workaround doesn't work in Clang under C23, so
+// try __builtin_nan first, if that exists.
+#ifdef _WIN32
+#ifdef __has_builtin
+#if __has_builtin(__builtin_nan)
+#define UPB_NAN __builtin_nan("0")
+#endif
+#if __has_builtin(__builtin_inf)
+#define UPB_INFINITY __builtin_inf()
+#endif
+#endif
+#ifndef UPB_NAN
+#define UPB_NAN 0.0 / 0.0
+#endif
+#ifndef UPB_INFINITY
+#define UPB_INFINITY 1.0 / 0.0
+#endif
+#else
+// For !_WIN32, assume math.h works.
+#define UPB_NAN NAN
+#define UPB_INFINITY INFINITY
+#endif
+
+const float kUpb_FltInfinity = UPB_INFINITY;
+const double kUpb_Infinity = UPB_INFINITY;
+const double kUpb_NaN = UPB_NAN;
 
 bool UPB_PRIVATE(_upb_Message_Realloc)(struct upb_Message* msg, size_t need,
                                        upb_Arena* a) {
diff --git a/ruby/ext/google/protobuf_c/ruby-upb.c b/ruby/ext/google/protobuf_c/ruby-upb.c
@@ -5649,7 +5649,7 @@ upb_Array* upb_Array_DeepClone(const upb_Array* array, upb_CType value_type,
   for (size_t i = 0; i < size; ++i) {
     upb_MessageValue val = upb_Array_Get(array, i);
     if (!upb_Clone_MessageValue(&val, value_type, sub, arena)) {
-      return false;
+      return NULL;
     }
     upb_Array_Set(cloned_array, i, val);
   }
@@ -11692,9 +11692,35 @@ upb_Extension* UPB_PRIVATE(_upb_Message_GetOrCreateExtension)(
 
 // Must be last.
 
-const float kUpb_FltInfinity = INFINITY;
-const double kUpb_Infinity = INFINITY;
-const double kUpb_NaN = NAN;
+// The latest win32 SDKs have an invalid definition of NAN.
+// https://developercommunity.visualstudio.com/t/NAN-is-no-longer-compile-time-constant-i/10688907
+//
+// Unfortunately, the `0.0 / 0.0` workaround doesn't work in Clang under C23, so
+// try __builtin_nan first, if that exists.
+#ifdef _WIN32
+#ifdef __has_builtin
+#if __has_builtin(__builtin_nan)
+#define UPB_NAN __builtin_nan("0")
+#endif
+#if __has_builtin(__builtin_inf)
+#define UPB_INFINITY __builtin_inf()
+#endif
+#endif
+#ifndef UPB_NAN
+#define UPB_NAN 0.0 / 0.0
+#endif
+#ifndef UPB_INFINITY
+#define UPB_INFINITY 1.0 / 0.0
+#endif
+#else
+// For !_WIN32, assume math.h works.
+#define UPB_NAN NAN
+#define UPB_INFINITY INFINITY
+#endif
+
+const float kUpb_FltInfinity = UPB_INFINITY;
+const double kUpb_Infinity = UPB_INFINITY;
+const double kUpb_NaN = UPB_NAN;
 
 bool UPB_PRIVATE(_upb_Message_Realloc)(struct upb_Message* msg, size_t need,
                                        upb_Arena* a) {
diff --git a/upb/message/copy.c b/upb/message/copy.c
@@ -150,7 +150,7 @@ upb_Array* upb_Array_DeepClone(const upb_Array* array, upb_CType value_type,
   for (size_t i = 0; i < size; ++i) {
     upb_MessageValue val = upb_Array_Get(array, i);
     if (!upb_Clone_MessageValue(&val, value_type, sub, arena)) {
-      return false;
+      return NULL;
     }
     upb_Array_Set(cloned_array, i, val);
   }
diff --git a/upb/message/internal/message.c b/upb/message/internal/message.c
@@ -17,9 +17,35 @@
 // Must be last.
 #include "upb/port/def.inc"
 
-const float kUpb_FltInfinity = INFINITY;
-const double kUpb_Infinity = INFINITY;
-const double kUpb_NaN = NAN;
+// The latest win32 SDKs have an invalid definition of NAN.
+// https://developercommunity.visualstudio.com/t/NAN-is-no-longer-compile-time-constant-i/10688907
+//
+// Unfortunately, the `0.0 / 0.0` workaround doesn't work in Clang under C23, so
+// try __builtin_nan first, if that exists.
+#ifdef _WIN32
+#ifdef __has_builtin
+#if __has_builtin(__builtin_nan)
+#define UPB_NAN __builtin_nan("0")
+#endif
+#if __has_builtin(__builtin_inf)
+#define UPB_INFINITY __builtin_inf()
+#endif
+#endif
+#ifndef UPB_NAN
+#define UPB_NAN 0.0 / 0.0
+#endif
+#ifndef UPB_INFINITY
+#define UPB_INFINITY 1.0 / 0.0
+#endif
+#else
+// For !_WIN32, assume math.h works.
+#define UPB_NAN NAN
+#define UPB_INFINITY INFINITY
+#endif
+
+const float kUpb_FltInfinity = UPB_INFINITY;
+const double kUpb_Infinity = UPB_INFINITY;
+const double kUpb_NaN = UPB_NAN;
 
 bool UPB_PRIVATE(_upb_Message_Realloc)(struct upb_Message* msg, size_t need,
                                        upb_Arena* a) {
diff --git a/upb/text/internal/encode.c b/upb/text/internal/encode.c
@@ -22,11 +22,11 @@
 // Must be last.
 #include "upb/port/def.inc"
 
-#define CHK(x)      \
-  do {              \
-    if (!(x)) {     \
-      return false; \
-    }               \
+#define CHK(x)     \
+  do {             \
+    if (!(x)) {    \
+      return NULL; \
+    }              \
   } while (0)
 
 /*
@@ -177,4 +177,4 @@ void UPB_PRIVATE(_upb_TextEncode_Scalar)(txtenc* e, upb_MessageValue val,
     default:
       UPB_UNREACHABLE();
   }
-}
+}

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`# This information is extracted from the MacOS runner specs located at:`
`2`		`-# https://github.com/actions/runner-images/blob/main/images/macos/macos-12-Readme.md`
	`2`	`+# https://github.com/actions/runner-images/blob/main/images/macos/macos-13-Readme.md`
`3`	`3`	`#`
`4`	`4`	`# When updating, also ensure the "xcode_destination" entries in`
`5`	`5`	# `.github/workflows/test_objectivec.yml` are supported for the given versions
Original file line number	Diff line number	Diff line change
`@@ -150,7 +150,7 @@ upb_Array* upb_Array_DeepClone(const upb_Array* array, upb_CType value_type,`
`150`	`150`	`for (size_t i = 0; i < size; ++i) {`
`151`	`151`	`upb_MessageValue val = upb_Array_Get(array, i);`
`152`	`152`	`if (!upb_Clone_MessageValue(&val, value_type, sub, arena)) {`
`153`		`- return false;`
	`153`	`+ return NULL;`
`154`	`154`	`}`
`155`	`155`	`upb_Array_Set(cloned_array, i, val);`
`156`	`156`	`}`