From f877215148b5df8b5cd123b0c635971e97e637d1 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Thu, 17 Apr 2025 19:55:21 -0400 Subject: [PATCH] ByteString: Adjust bitfields for MSVC Microsoft Visual C++ compiler has more restrictions than usual about bit fields. There are two restrictions that particularly break ByteString: - A consecutive field with a different type will force alignment to next boundary - The alignment boundary is based on the type of the bitfield Because of this, we can't use the enumeration type inside of the bitfield. Actually though, this is mostly OK: the header struct is the only one that actually needs a typed enum, and nothing follows it in the header struct. Therefore, this workaround should make all of GCC, Clang and MSVC happy. Also: fixes the #pragma pack directives, which are typo'd slightly. --- common/internal/byte_string.cc | 2 +- common/internal/byte_string.h | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/common/internal/byte_string.cc b/common/internal/byte_string.cc index 7d588bb57..f7891e6dd 100644 --- a/common/internal/byte_string.cc +++ b/common/internal/byte_string.cc @@ -97,7 +97,7 @@ ByteString ByteString::Concat(const ByteString& lhs, const ByteString& rhs, result.rep_.medium.size = result_size; result.rep_.medium.owner = reinterpret_cast(arena) | kMetadataOwnerArenaBit; - result.rep_.medium.kind = ByteStringKind::kMedium; + result.rep_.header.kind = ByteStringKind::kMedium; } return result; } diff --git a/common/internal/byte_string.h b/common/internal/byte_string.h index 8dee78842..a95ba9517 100644 --- a/common/internal/byte_string.h +++ b/common/internal/byte_string.h @@ -78,14 +78,14 @@ inline std::ostream& operator<<(std::ostream& out, ByteStringKind kind) { // Representation of small strings in ByteString, which are stored in place. struct CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI SmallByteStringRep final { #ifdef _MSC_VER -#pragma push(pack, 1) +#pragma pack(push, 1) #endif struct ABSL_ATTRIBUTE_PACKED CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI { - ByteStringKind kind : 2; - size_t size : 6; + std::uint8_t kind : 2; + std::uint8_t size : 6; }; #ifdef _MSC_VER -#pragma pop(pack) +#pragma pack(pop) #endif char data[23 - sizeof(google::protobuf::Arena*)]; google::protobuf::Arena* ABSL_NULLABLE arena; @@ -107,14 +107,14 @@ inline constexpr size_t kByteStringViewMaxSize = // the same semantics as `cel::Owner`. struct CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI MediumByteStringRep final { #ifdef _MSC_VER -#pragma push(pack, 1) +#pragma pack(push, 1) #endif struct ABSL_ATTRIBUTE_PACKED CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI { - ByteStringKind kind : 2; + size_t kind : 2; size_t size : kMediumByteStringSizeBits; }; #ifdef _MSC_VER -#pragma pop(pack) +#pragma pack(pop) #endif const char* data; uintptr_t owner; @@ -124,14 +124,14 @@ struct CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI MediumByteStringRep final { // `absl::Cord` and never owned by an arena. struct CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI LargeByteStringRep final { #ifdef _MSC_VER -#pragma push(pack, 1) +#pragma pack(push, 1) #endif struct ABSL_ATTRIBUTE_PACKED CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI { - ByteStringKind kind : 2; + size_t kind : 2; size_t padding : kMediumByteStringSizeBits; }; #ifdef _MSC_VER -#pragma pop(pack) +#pragma pack(pop) #endif alignas(absl::Cord) std::byte data[sizeof(absl::Cord)]; }; @@ -139,13 +139,13 @@ struct CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI LargeByteStringRep final { // Representation of ByteString. union CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI ByteStringRep final { #ifdef _MSC_VER -#pragma push(pack, 1) +#pragma pack(push, 1) #endif struct ABSL_ATTRIBUTE_PACKED CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI { ByteStringKind kind : 2; } header; #ifdef _MSC_VER -#pragma pop(pack) +#pragma pack(pop) #endif SmallByteStringRep small; MediumByteStringRep medium;