Fixed the upb.lexan build:

- Fixed a couple of broken tests that were probably invoking UB.
- Excluded python/... and js/..., as these do not work with Windows.

PiperOrigin-RevId: 525228589

BUILD

@@ -682,11 +682,6 @@ cc_test(
         "upb/reflection/def_builder_test.cc",
         "upb/reflection/def_type.h",
     ],
-    # TODO(b/259158612): fix this test on Windows.
-    target_compatible_with = select({
-        "@platforms//os:windows": ["@platforms//:incompatible"],
-        "//conditions:default": [],
-    }),
     deps = [
         ":descriptor_upb_proto",
         ":hash",
@@ -694,6 +689,7 @@ cc_test(
         ":reflection",
         ":reflection_internal",
         ":upb",
+        "@com_google_absl//absl/strings",
         "@com_google_googletest//:gtest_main",
     ],
 )

upb/reflection/def_builder_test.cc

@@ -26,33 +26,53 @@
  */
 
 #include "gtest/gtest.h"
-#include "upb/reflection/def.hpp"
+#include "absl/strings/string_view.h"
 #include "upb/reflection/def_builder_internal.h"
+#include "upb/upb.hpp"
 
 // Must be last.
 #include "upb/port/def.inc"
 
-struct IdentTest {
+struct IdentTestData {
-  const char* text;
+  absl::string_view text;
   bool ok;
 };
 
-static const std::vector<IdentTest> FullIdentTests = {
+class FullIdentTestBase : public testing::TestWithParam<IdentTestData> {};
-    {"foo.bar", true},   {"foo.", true},  {"foo", true},
 
-    {"foo.7bar", false}, {".foo", false}, {"#", false},
+TEST_P(FullIdentTestBase, CheckFullIdent) {
-    {".", false},        {"", false},
+  upb_Status status;
-};
+  upb_DefBuilder ctx;
+  upb::Arena arena;
+  ctx.status = &status;
+  ctx.arena = arena.ptr();
+  upb_Status_Clear(&status);
+
+  if (UPB_SETJMP(ctx.err)) {
+    EXPECT_FALSE(GetParam().ok);
+  } else {
+    _upb_DefBuilder_CheckIdentFull(
+        &ctx, upb_StringView_FromDataAndSize(GetParam().text.data(),
+                                             GetParam().text.size()));
+    EXPECT_TRUE(GetParam().ok);
+  }
+}
 
-static const std::vector<IdentTest> NotFullIdentTests = {
+INSTANTIATE_TEST_SUITE_P(FullIdentTest, FullIdentTestBase,
-    {"foo", true},      {"foo1", true},
+                         testing::ValuesIn(std::vector<IdentTestData>{
+                             {"foo.bar", true},
+                             {"foo.", true},
+                             {"foo", true},
 
-    {"foo.bar", false}, {"1foo", false}, {"#", false},
+                             {"foo.7bar", false},
-    {".", false},       {"", false},
+                             {".foo", false},
-};
+                             {"#", false},
+                             {".", false},
+                             {"", false}}));
+
+class PartIdentTestBase : public testing::TestWithParam<IdentTestData> {};
 
-TEST(DefBuilder, TestIdents) {
+TEST_P(PartIdentTestBase, TestNotFullIdent) {
-  upb_StringView sv;
   upb_Status status;
   upb_DefBuilder ctx;
   upb::Arena arena;
@@ -60,27 +80,24 @@ TEST(DefBuilder, TestIdents) {
   ctx.arena = arena.ptr();
   upb_Status_Clear(&status);
 
-  for (const auto& test : FullIdentTests) {
+  if (UPB_SETJMP(ctx.err)) {
-    sv.data = test.text;
+    EXPECT_FALSE(GetParam().ok);
-    sv.size = strlen(test.text);
+  } else {
-
+    _upb_DefBuilder_MakeFullName(
-    if (UPB_SETJMP(ctx.err)) {
+        &ctx, "abc",
-      EXPECT_FALSE(test.ok);
+        upb_StringView_FromDataAndSize(GetParam().text.data(),
-    } else {
+                                       GetParam().text.size()));
-      _upb_DefBuilder_CheckIdentFull(&ctx, sv);
+    EXPECT_TRUE(GetParam().ok);
-      EXPECT_TRUE(test.ok);
-    }
   }
+}
 
-  for (const auto& test : NotFullIdentTests) {
+INSTANTIATE_TEST_SUITE_P(PartIdentTest, PartIdentTestBase,
-    sv.data = test.text;
+                         testing::ValuesIn(std::vector<IdentTestData>{
-    sv.size = strlen(test.text);
+                             {"foo", true},
+                             {"foo1", true},
 
-    if (UPB_SETJMP(ctx.err)) {
+                             {"foo.bar", false},
-      EXPECT_FALSE(test.ok);
+                             {"1foo", false},
-    } else {
+                             {"#", false},
-      _upb_DefBuilder_MakeFullName(&ctx, "abc", sv);
+                             {".", false},
-      EXPECT_TRUE(test.ok);
+                             {"", false}}));
-    }
-  }
-}

upb/util/BUILD

@@ -151,11 +151,6 @@ cc_library(
 cc_test(
     name = "compare_test",
     srcs = ["compare_test.cc"],
-    # TODO(b/259158757): fix this test on Windows.
-    target_compatible_with = select({
-        "@platforms//os:windows": ["@platforms//:incompatible"],
-        "//conditions:default": [],
-    }),
     deps = [
         ":compare",
         "//:wire_internal",

upb/util/compare_test.cc

@@ -29,12 +29,12 @@
 
 #include <stdint.h>
 
-#include <string_view>
+#include <initializer_list>
+#include <string>
+#include <variant>
 #include <vector>
 
-#include "gmock/gmock.h"
 #include "gtest/gtest.h"
-#include "absl/strings/string_view.h"
 #include "upb/wire/swap_internal.h"
 #include "upb/wire/types.h"
 
@@ -42,70 +42,36 @@ struct UnknownField;
 
 using UnknownFields = std::vector<UnknownField>;
 
-enum class UnknownFieldType {
+struct Varint {
-  kVarint,
+  explicit Varint(uint64_t _val) : val(_val) {}
-  kLongVarint,  // Over-encoded to have distinct wire format.
+  uint64_t val;
-  kDelimited,
-  kFixed64,
-  kFixed32,
-  kGroup,
 };
-
+struct LongVarint {
-union UnknownFieldValue {
+  explicit LongVarint(uint64_t _val) : val(_val) {}
-  uint64_t varint;
+  uint64_t val;  // Over-encoded.
-  uint64_t fixed64;
-  uint32_t fixed32;
-  // NULL-terminated (strings must not have embedded NULL).
-  const char* delimited;
-  UnknownFields* group;
 };
-
+struct Delimited {
-struct TypeAndValue {
+  explicit Delimited(std::string _val) : val(_val) {}
-  UnknownFieldType type;
+  std::string val;
-  UnknownFieldValue value;
+};
+struct Fixed64 {
+  explicit Fixed64(uint64_t _val) : val(_val) {}
+  uint64_t val;
+};
+struct Fixed32 {
+  explicit Fixed32(uint32_t _val) : val(_val) {}
+  uint32_t val;
+};
+struct Group {
+  Group(std::initializer_list<UnknownField> _val) : val(_val) {}
+  UnknownFields val;
 };
 
 struct UnknownField {
   uint32_t field_number;
-  TypeAndValue value;
+  std::variant<Varint, LongVarint, Delimited, Fixed64, Fixed32, Group> value;
 };
 
-TypeAndValue Varint(uint64_t val) {
-  TypeAndValue ret{UnknownFieldType::kVarint};
-  ret.value.varint = val;
-  return ret;
-}
-
-TypeAndValue LongVarint(uint64_t val) {
-  TypeAndValue ret{UnknownFieldType::kLongVarint};
-  ret.value.varint = val;
-  return ret;
-}
-
-TypeAndValue Fixed64(uint64_t val) {
-  TypeAndValue ret{UnknownFieldType::kFixed64};
-  ret.value.fixed64 = val;
-  return ret;
-}
-
-TypeAndValue Fixed32(uint32_t val) {
-  TypeAndValue ret{UnknownFieldType::kFixed32};
-  ret.value.fixed32 = val;
-  return ret;
-}
-
-TypeAndValue Delimited(const char* val) {
-  TypeAndValue ret{UnknownFieldType::kDelimited};
-  ret.value.delimited = val;
-  return ret;
-}
-
-TypeAndValue Group(UnknownFields nested) {
-  TypeAndValue ret{UnknownFieldType::kGroup};
-  ret.value.group = &nested;
-  return ret;
-}
-
 void EncodeVarint(uint64_t val, std::string* str) {
   do {
     char byte = val & 0x7fU;
@@ -116,45 +82,33 @@ void EncodeVarint(uint64_t val, std::string* str) {
 }
 
 std::string ToBinaryPayload(const UnknownFields& fields) {
-  static const upb_WireType wire_types[] = {
-      kUpb_WireType_Varint, kUpb_WireType_Varint, kUpb_WireType_Delimited,
-      kUpb_WireType_64Bit,  kUpb_WireType_32Bit,  kUpb_WireType_StartGroup,
-  };
   std::string ret;
 
   for (const auto& field : fields) {
-    uint32_t tag = field.field_number << 3 |
+    if (const auto* val = std::get_if<Varint>(&field.value)) {
-                   (wire_types[static_cast<int>(field.value.type)]);
+      EncodeVarint(field.field_number << 3 | kUpb_WireType_Varint, &ret);
-    EncodeVarint(tag, &ret);
+      EncodeVarint(val->val, &ret);
-    switch (field.value.type) {
+    } else if (const auto* val = std::get_if<LongVarint>(&field.value)) {
-      case UnknownFieldType::kVarint:
+      EncodeVarint(field.field_number << 3 | kUpb_WireType_Varint, &ret);
-        EncodeVarint(field.value.value.varint, &ret);
+      EncodeVarint(val->val, &ret);
-        break;
+      ret.back() |= 0x80;
-      case UnknownFieldType::kLongVarint:
+      ret.push_back(0);
-        EncodeVarint(field.value.value.varint, &ret);
+    } else if (const auto* val = std::get_if<Delimited>(&field.value)) {
-        ret.back() |= 0x80;
+      EncodeVarint(field.field_number << 3 | kUpb_WireType_Delimited, &ret);
-        ret.push_back(0);
+      EncodeVarint(val->val.size(), &ret);
-        break;
+      ret.append(val->val);
-      case UnknownFieldType::kDelimited:
+    } else if (const auto* val = std::get_if<Fixed64>(&field.value)) {
-        EncodeVarint(strlen(field.value.value.delimited), &ret);
+      EncodeVarint(field.field_number << 3 | kUpb_WireType_64Bit, &ret);
-        ret.append(field.value.value.delimited);
+      uint64_t swapped = _upb_BigEndian_Swap64(val->val);
-        break;
+      ret.append(reinterpret_cast<const char*>(&swapped), sizeof(swapped));
-      case UnknownFieldType::kFixed64: {
+    } else if (const auto* val = std::get_if<Fixed32>(&field.value)) {
-        uint64_t val = _upb_BigEndian_Swap64(field.value.value.fixed64);
+      EncodeVarint(field.field_number << 3 | kUpb_WireType_32Bit, &ret);
-        ret.append(reinterpret_cast<const char*>(&val), sizeof(val));
+      uint32_t swapped = _upb_BigEndian_Swap32(val->val);
-        break;
+      ret.append(reinterpret_cast<const char*>(&swapped), sizeof(swapped));
-      }
+    } else if (const auto* val = std::get_if<Group>(&field.value)) {
-      case UnknownFieldType::kFixed32: {
+      EncodeVarint(field.field_number << 3 | kUpb_WireType_StartGroup, &ret);
-        uint32_t val = _upb_BigEndian_Swap32(field.value.value.fixed32);
+      ret.append(ToBinaryPayload(val->val));
-        ret.append(reinterpret_cast<const char*>(&val), sizeof(val));
+      EncodeVarint(field.field_number << 3 | kUpb_WireType_EndGroup, &ret);
-        break;
-      }
-      case UnknownFieldType::kGroup: {
-        uint32_t end_tag = field.field_number << 3 | kUpb_WireType_EndGroup;
-        ret.append(ToBinaryPayload(*field.value.value.group));
-        EncodeVarint(end_tag, &ret);
-        break;
-      }
     }
   }