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
diff --git a/BUILD b/BUILD
index e421be7..045ded0 100644
--- a/BUILD
+++ b/BUILD
@@ -682,11 +682,6 @@
"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 @@
":reflection",
":reflection_internal",
":upb",
+ "@com_google_absl//absl/strings",
"@com_google_googletest//:gtest_main",
],
)
diff --git a/upb/reflection/def_builder_test.cc b/upb/reflection/def_builder_test.cc
index b57ae28..1166dd2 100644
--- a/upb/reflection/def_builder_test.cc
+++ b/upb/reflection/def_builder_test.cc
@@ -26,33 +26,21 @@
*/
#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 {
- const char* text;
+struct IdentTestData {
+ absl::string_view text;
bool ok;
};
-static const std::vector<IdentTest> FullIdentTests = {
- {"foo.bar", true}, {"foo.", true}, {"foo", true},
+class FullIdentTestBase : public testing::TestWithParam<IdentTestData> {};
- {"foo.7bar", false}, {".foo", false}, {"#", false},
- {".", false}, {"", false},
-};
-
-static const std::vector<IdentTest> NotFullIdentTests = {
- {"foo", true}, {"foo1", true},
-
- {"foo.bar", false}, {"1foo", false}, {"#", false},
- {".", false}, {"", false},
-};
-
-TEST(DefBuilder, TestIdents) {
- upb_StringView sv;
+TEST_P(FullIdentTestBase, CheckFullIdent) {
upb_Status status;
upb_DefBuilder ctx;
upb::Arena arena;
@@ -60,27 +48,56 @@
ctx.arena = arena.ptr();
upb_Status_Clear(&status);
- for (const auto& test : FullIdentTests) {
- sv.data = test.text;
- sv.size = strlen(test.text);
-
- if (UPB_SETJMP(ctx.err)) {
- EXPECT_FALSE(test.ok);
- } else {
- _upb_DefBuilder_CheckIdentFull(&ctx, sv);
- EXPECT_TRUE(test.ok);
- }
- }
-
- for (const auto& test : NotFullIdentTests) {
- sv.data = test.text;
- sv.size = strlen(test.text);
-
- if (UPB_SETJMP(ctx.err)) {
- EXPECT_FALSE(test.ok);
- } else {
- _upb_DefBuilder_MakeFullName(&ctx, "abc", sv);
- EXPECT_TRUE(test.ok);
- }
+ 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);
}
}
+
+INSTANTIATE_TEST_SUITE_P(FullIdentTest, FullIdentTestBase,
+ testing::ValuesIn(std::vector<IdentTestData>{
+ {"foo.bar", true},
+ {"foo.", true},
+ {"foo", true},
+
+ {"foo.7bar", false},
+ {".foo", false},
+ {"#", false},
+ {".", false},
+ {"", false}}));
+
+class PartIdentTestBase : public testing::TestWithParam<IdentTestData> {};
+
+TEST_P(PartIdentTestBase, TestNotFullIdent) {
+ 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_MakeFullName(
+ &ctx, "abc",
+ upb_StringView_FromDataAndSize(GetParam().text.data(),
+ GetParam().text.size()));
+ EXPECT_TRUE(GetParam().ok);
+ }
+}
+
+INSTANTIATE_TEST_SUITE_P(PartIdentTest, PartIdentTestBase,
+ testing::ValuesIn(std::vector<IdentTestData>{
+ {"foo", true},
+ {"foo1", true},
+
+ {"foo.bar", false},
+ {"1foo", false},
+ {"#", false},
+ {".", false},
+ {"", false}}));
diff --git a/upb/util/BUILD b/upb/util/BUILD
index 5c5a7ee..7948bcf 100644
--- a/upb/util/BUILD
+++ b/upb/util/BUILD
@@ -151,11 +151,6 @@
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",
diff --git a/upb/util/compare_test.cc b/upb/util/compare_test.cc
index 25e7b2d..e168f70 100644
--- a/upb/util/compare_test.cc
+++ b/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 @@
using UnknownFields = std::vector<UnknownField>;
-enum class UnknownFieldType {
- kVarint,
- kLongVarint, // Over-encoded to have distinct wire format.
- kDelimited,
- kFixed64,
- kFixed32,
- kGroup,
+struct Varint {
+ explicit Varint(uint64_t _val) : val(_val) {}
+ uint64_t val;
};
-
-union UnknownFieldValue {
- uint64_t varint;
- uint64_t fixed64;
- uint32_t fixed32;
- // NULL-terminated (strings must not have embedded NULL).
- const char* delimited;
- UnknownFields* group;
+struct LongVarint {
+ explicit LongVarint(uint64_t _val) : val(_val) {}
+ uint64_t val; // Over-encoded.
};
-
-struct TypeAndValue {
- UnknownFieldType type;
- UnknownFieldValue value;
+struct Delimited {
+ explicit Delimited(std::string _val) : val(_val) {}
+ std::string val;
+};
+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 @@
}
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 |
- (wire_types[static_cast<int>(field.value.type)]);
- EncodeVarint(tag, &ret);
- switch (field.value.type) {
- case UnknownFieldType::kVarint:
- EncodeVarint(field.value.value.varint, &ret);
- break;
- case UnknownFieldType::kLongVarint:
- EncodeVarint(field.value.value.varint, &ret);
- ret.back() |= 0x80;
- ret.push_back(0);
- break;
- case UnknownFieldType::kDelimited:
- EncodeVarint(strlen(field.value.value.delimited), &ret);
- ret.append(field.value.value.delimited);
- break;
- case UnknownFieldType::kFixed64: {
- uint64_t val = _upb_BigEndian_Swap64(field.value.value.fixed64);
- ret.append(reinterpret_cast<const char*>(&val), sizeof(val));
- break;
- }
- case UnknownFieldType::kFixed32: {
- uint32_t val = _upb_BigEndian_Swap32(field.value.value.fixed32);
- ret.append(reinterpret_cast<const char*>(&val), sizeof(val));
- 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;
- }
+ if (const auto* val = std::get_if<Varint>(&field.value)) {
+ EncodeVarint(field.field_number << 3 | kUpb_WireType_Varint, &ret);
+ EncodeVarint(val->val, &ret);
+ } else if (const auto* val = std::get_if<LongVarint>(&field.value)) {
+ EncodeVarint(field.field_number << 3 | kUpb_WireType_Varint, &ret);
+ EncodeVarint(val->val, &ret);
+ ret.back() |= 0x80;
+ ret.push_back(0);
+ } else if (const auto* val = std::get_if<Delimited>(&field.value)) {
+ EncodeVarint(field.field_number << 3 | kUpb_WireType_Delimited, &ret);
+ EncodeVarint(val->val.size(), &ret);
+ ret.append(val->val);
+ } else if (const auto* val = std::get_if<Fixed64>(&field.value)) {
+ EncodeVarint(field.field_number << 3 | kUpb_WireType_64Bit, &ret);
+ uint64_t swapped = _upb_BigEndian_Swap64(val->val);
+ ret.append(reinterpret_cast<const char*>(&swapped), sizeof(swapped));
+ } else if (const auto* val = std::get_if<Fixed32>(&field.value)) {
+ EncodeVarint(field.field_number << 3 | kUpb_WireType_32Bit, &ret);
+ uint32_t swapped = _upb_BigEndian_Swap32(val->val);
+ ret.append(reinterpret_cast<const char*>(&swapped), sizeof(swapped));
+ } else if (const auto* val = std::get_if<Group>(&field.value)) {
+ EncodeVarint(field.field_number << 3 | kUpb_WireType_StartGroup, &ret);
+ ret.append(ToBinaryPayload(val->val));
+ EncodeVarint(field.field_number << 3 | kUpb_WireType_EndGroup, &ret);
}
}
