diff --git a/common/BUILD b/common/BUILD index f34f07fcf..81c902ff0 100644 --- a/common/BUILD +++ b/common/BUILD @@ -789,7 +789,6 @@ cc_library( ":value_kind", "//base:attributes", "//common/internal:byte_string", - "//common/internal:reference_count", "//eval/internal:cel_value_equal", "//eval/public:cel_value", "//eval/public:message_wrapper", diff --git a/common/internal/BUILD b/common/internal/BUILD index 3be350754..eebf66219 100644 --- a/common/internal/BUILD +++ b/common/internal/BUILD @@ -67,11 +67,6 @@ cc_library( srcs = ["byte_string.cc"], hdrs = ["byte_string.h"], deps = [ - ":metadata", - ":reference_count", - "//common:allocator", - "//common:arena", - "//common:memory", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/functional:overload", @@ -79,6 +74,7 @@ cc_library( "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:cord", + "@com_google_absl//absl/strings:resize_and_overwrite", "@com_google_absl//absl/strings:string_view", "@com_google_absl//absl/types:optional", "@com_google_protobuf//:protobuf", @@ -90,16 +86,12 @@ cc_test( srcs = ["byte_string_test.cc"], deps = [ ":byte_string", - ":reference_count", - "//common:allocator", - "//common:memory", "//internal:testing", "@com_google_absl//absl/base:no_destructor", "@com_google_absl//absl/hash", "@com_google_absl//absl/strings:cord", "@com_google_absl//absl/strings:cord_test_helpers", "@com_google_absl//absl/strings:string_view", - "@com_google_absl//absl/types:optional", "@com_google_protobuf//:protobuf", ], ) diff --git a/common/internal/byte_string.cc b/common/internal/byte_string.cc index 201842a10..468bf893a 100644 --- a/common/internal/byte_string.cc +++ b/common/internal/byte_string.cc @@ -14,11 +14,11 @@ #include "common/internal/byte_string.h" +#include #include #include #include #include -#include #include #include "absl/base/nullability.h" @@ -28,26 +28,62 @@ #include "absl/log/absl_check.h" #include "absl/strings/cord.h" #include "absl/strings/match.h" +#include "absl/strings/resize_and_overwrite.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" -#include "common/allocator.h" -#include "common/internal/metadata.h" -#include "common/internal/reference_count.h" -#include "common/memory.h" #include "google/protobuf/arena.h" namespace cel::common_internal { namespace { -char* CopyCordToArray(const absl::Cord& cord, char* data) { +char* CopyCordToArray(const absl::Cord& cord, size_t offset, size_t size, + char* data) { for (auto chunk : cord.Chunks()) { - std::memcpy(data, chunk.data(), chunk.size()); - data += chunk.size(); + if (size == 0) { + break; + } + if (offset > 0) { + size_t min_offset = std::min(chunk.size(), offset); + offset -= min_offset; + if (offset > 0) { + continue; + } + chunk.remove_prefix(min_offset); + } + size_t min_size = std::min(size, chunk.size()); + std::memcpy(data, chunk.data(), min_size); + data += min_size; + size -= min_size; } return data; } +char* CopyCordToArray(const absl::Cord& cord, char* data) { + return (CopyCordToArray)(cord, 0, cord.size(), data); +} + +void AppendCordToString(const absl::Cord& cord, size_t offset, size_t size, + std::string& data) { + data.reserve(data.size() + size); + for (auto chunk : cord.Chunks()) { + if (size == 0) { + break; + } + if (offset > 0) { + size_t min_offset = std::min(chunk.size(), offset); + offset -= min_offset; + if (offset > 0) { + continue; + } + chunk.remove_prefix(min_offset); + } + size_t min_size = std::min(size, chunk.size()); + data.append(absl::string_view(chunk.data(), min_size)); + size -= min_size; + } +} + template T ConsumeAndDestroy(T& object) { T consumed = std::move(object); @@ -57,6 +93,80 @@ T ConsumeAndDestroy(T& object) { } // namespace +ByteString ByteString::From(const char* absl_nullable value, + google::protobuf::Arena* absl_nonnull arena) { + return From(absl::NullSafeStringView(value), arena); +} + +ByteString ByteString::From(absl::string_view value, + google::protobuf::Arena* absl_nonnull arena) { + ABSL_DCHECK(arena != nullptr); + ByteString result(UninitializedTag{}); + if (value.size() <= kSmallByteStringCapacity) { + result.SetSmall(arena, value); + } else { + char* arena_value = + reinterpret_cast(arena->AllocateAligned(value.size())); + std::memcpy(arena_value, value.data(), value.size()); + result.SetMedium(arena, absl::string_view(arena_value, value.size())); + } + return result; +} + +ByteString ByteString::From(const absl::Cord& value, + google::protobuf::Arena* absl_nonnull arena) { + ABSL_DCHECK(arena != nullptr); + ByteString result(UninitializedTag{}); + if (value.size() <= kSmallByteStringCapacity) { + result.SetSmall(arena, value); + } else { + result.SetLarge(arena, google::protobuf::Arena::Create(arena, value)); + } + return result; +} + +ByteString ByteString::From(std::string&& value, + google::protobuf::Arena* absl_nonnull arena) { + ABSL_DCHECK(arena != nullptr); + ByteString result(UninitializedTag{}); + if (value.size() <= kSmallByteStringCapacity) { + result.SetSmall(arena, value); + } else if (value.size() > sizeof(std::string)) { + value.shrink_to_fit(); + result.SetMedium( + arena, google::protobuf::Arena::Create(arena, std::move(value))); + } else { + char* arena_value = + reinterpret_cast(arena->AllocateAligned(value.size())); + std::memcpy(arena_value, value.data(), value.size()); + result.SetMedium(arena, absl::string_view(arena_value, value.size())); + } + return result; +} + +ByteString ByteString::Wrap(absl::string_view value, + google::protobuf::Arena* absl_nullable arena) { + ByteString result(UninitializedTag{}); + result.SetMedium(arena, value); + return result; +} + +ByteString ByteString::Wrap(const absl::Cord* absl_nonnull value, size_t offset, + size_t size, google::protobuf::Arena* absl_nullable arena) { + ByteString result(UninitializedTag{}); + result.SetLarge(arena, value, offset, size); + return result; +} + +ByteString ByteString::WrapUnsafe(absl::string_view value) { + return Wrap(value, static_cast(nullptr)); +} + +ByteString ByteString::WrapUnsafe(const absl::Cord* absl_nonnull value, + size_t offset, size_t size) { + return Wrap(value, offset, size, static_cast(nullptr)); +} + ByteString ByteString::Concat(const ByteString& lhs, const ByteString& rhs, google::protobuf::Arena* absl_nonnull arena) { ABSL_DCHECK(arena != nullptr); @@ -72,17 +182,18 @@ ByteString ByteString::Concat(const ByteString& lhs, const ByteString& rhs, rhs.GetKind() == ByteStringKind::kLarge) { // If either the left or right are absl::Cord, use absl::Cord. absl::Cord result; - result.Append(lhs.ToCord()); - result.Append(rhs.ToCord()); - return ByteString(std::move(result)); + lhs.AppendToCord(&result); + rhs.AppendToCord(&result); + return From(result, arena); } const size_t lhs_size = lhs.size(); const size_t rhs_size = rhs.size(); const size_t result_size = lhs_size + rhs_size; - ByteString result; + ByteString result(UninitializedTag{}); if (result_size <= kSmallByteStringCapacity) { // If the resulting string fits in inline storage, do it. + result.rep_.header.kind = ByteStringKind::kSmall; result.rep_.small.size = result_size; result.rep_.small.arena = arena; lhs.CopyToArray(result.rep_.small.data); @@ -93,100 +204,14 @@ ByteString ByteString::Concat(const ByteString& lhs, const ByteString& rhs, reinterpret_cast(arena->AllocateAligned(result_size)); lhs.CopyToArray(result_data); rhs.CopyToArray(result_data + lhs_size); + result.rep_.header.kind = ByteStringKind::kMedium; result.rep_.medium.data = result_data; result.rep_.medium.size = result_size; - result.rep_.medium.owner = - reinterpret_cast(arena) | kMetadataOwnerArenaBit; - result.rep_.header.kind = ByteStringKind::kMedium; + result.rep_.medium.arena = arena; } return result; } -ByteString::ByteString(Allocator<> allocator, absl::string_view string) { - ABSL_DCHECK_LE(string.size(), max_size()); - auto* arena = allocator.arena(); - if (string.size() <= kSmallByteStringCapacity) { - SetSmall(arena, string); - } else { - SetMedium(arena, string); - } -} - -ByteString::ByteString(Allocator<> allocator, const std::string& string) { - ABSL_DCHECK_LE(string.size(), max_size()); - auto* arena = allocator.arena(); - if (string.size() <= kSmallByteStringCapacity) { - SetSmall(arena, string); - } else { - SetMedium(arena, string); - } -} - -ByteString::ByteString(Allocator<> allocator, std::string&& string) { - ABSL_DCHECK_LE(string.size(), max_size()); - auto* arena = allocator.arena(); - if (string.size() <= kSmallByteStringCapacity) { - SetSmall(arena, string); - } else { - SetMedium(arena, std::move(string)); - } -} - -ByteString::ByteString(Allocator<> allocator, const absl::Cord& cord) { - ABSL_DCHECK_LE(cord.size(), max_size()); - auto* arena = allocator.arena(); - if (cord.size() <= kSmallByteStringCapacity) { - SetSmall(arena, cord); - } else if (arena != nullptr) { - SetMedium(arena, cord); - } else { - SetLarge(cord); - } -} - -ByteString ByteString::Borrowed(Borrower borrower, absl::string_view string) { - ABSL_DCHECK(borrower != Borrower::None()) << "Borrowing from Owner::None()"; - auto* arena = borrower.arena(); - if (string.size() <= kSmallByteStringCapacity || arena != nullptr) { - return ByteString(arena, string); - } - const auto* refcount = BorrowerRelease(borrower); - // A nullptr refcount indicates somebody called us to borrow something that - // has no owner. If this is the case, we fallback to assuming operator - // new/delete and convert it to a reference count. - if (refcount == nullptr) { - std::tie(refcount, string) = MakeReferenceCountedString(string); - } else { - StrongRef(*refcount); - } - return ByteString(refcount, string); -} - -ByteString ByteString::Borrowed(Borrower borrower, const absl::Cord& cord) { - ABSL_DCHECK(borrower != Borrower::None()) << "Borrowing from Owner::None()"; - return ByteString(borrower.arena(), cord); -} - -ByteString::ByteString(const ReferenceCount* absl_nonnull refcount, - absl::string_view string) { - ABSL_DCHECK_LE(string.size(), max_size()); - SetMedium(string, reinterpret_cast(refcount) | - kMetadataOwnerReferenceCountBit); -} - -ByteString::ByteString(ByteString::ExternalStringTag, - absl::string_view string) { - if (string.size() <= kSmallByteStringCapacity) { - SetSmall(nullptr, string); - } else { - SetExternalMedium(string); - } -} - -ByteString ByteString::FromExternal(absl::string_view string) { - return ByteString(ExternalStringTag{}, string); -} - google::protobuf::Arena* absl_nullable ByteString::GetArena() const { switch (GetKind()) { case ByteStringKind::kSmall: @@ -194,7 +219,7 @@ google::protobuf::Arena* absl_nullable ByteString::GetArena() const { case ByteStringKind::kMedium: return GetMediumArena(); case ByteStringKind::kLarge: - return nullptr; + return GetLargeArena(); } } @@ -205,7 +230,7 @@ bool ByteString::empty() const { case ByteStringKind::kMedium: return rep_.medium.size == 0; case ByteStringKind::kLarge: - return GetLarge().empty(); + return rep_.large.size == 0; } } @@ -216,18 +241,7 @@ size_t ByteString::size() const { case ByteStringKind::kMedium: return rep_.medium.size; case ByteStringKind::kLarge: - return GetLarge().size(); - } -} - -absl::string_view ByteString::Flatten() { - switch (GetKind()) { - case ByteStringKind::kSmall: - return GetSmall(); - case ByteStringKind::kMedium: - return GetMedium(); - case ByteStringKind::kLarge: - return GetLarge().Flatten(); + return rep_.large.size; } } @@ -237,8 +251,12 @@ absl::optional ByteString::TryFlat() const { return GetSmall(); case ByteStringKind::kMedium: return GetMedium(); - case ByteStringKind::kLarge: - return GetLarge().TryFlat(); + case ByteStringKind::kLarge: { + if (auto flat = rep_.large.data->TryFlat(); flat.has_value()) { + return flat->substr(rep_.large.offset, rep_.large.offset); + } + return absl::nullopt; + } } } @@ -380,22 +398,20 @@ ByteString ByteString::Substring(size_t pos, size_t npos) const { switch (GetKind()) { case ByteStringKind::kSmall: { - ByteString result; - result.rep_.header.kind = ByteStringKind::kSmall; - result.rep_.small.size = npos - pos; - std::memcpy(result.rep_.small.data, rep_.small.data + pos, - result.rep_.small.size); - result.rep_.small.arena = GetSmallArena(); + ByteString result(UninitializedTag{}); + result.SetSmall(GetSmallArena(), GetSmall().substr(pos, npos - pos)); return result; } case ByteStringKind::kMedium: { - ByteString result(*this); - result.rep_.medium.data += pos; - result.rep_.medium.size = npos - pos; + ByteString result(UninitializedTag{}); + result.SetMedium(GetMediumArena(), GetMedium().substr(pos, npos - pos)); return result; } case ByteStringKind::kLarge: - return ByteString(GetLarge().Subcord(pos, npos - pos)); + ByteString result(UninitializedTag{}); + result.SetLarge(GetLargeArena(), rep_.large.data, rep_.large.offset + pos, + npos - pos); + return result; } } @@ -407,29 +423,16 @@ void ByteString::RemovePrefix(size_t n) { switch (GetKind()) { case ByteStringKind::kSmall: std::memmove(rep_.small.data, rep_.small.data + n, rep_.small.size - n); - rep_.small.size -= n; + rep_.small.size = rep_.small.size - n; break; case ByteStringKind::kMedium: - rep_.medium.data += n; - rep_.medium.size -= n; - if (rep_.medium.size <= kSmallByteStringCapacity) { - const auto* refcount = GetMediumReferenceCount(); - SetSmall(GetMediumArena(), GetMedium()); - StrongUnref(refcount); - } + rep_.medium.data = rep_.medium.data + n; + rep_.medium.size = rep_.medium.size - n; + break; + case ByteStringKind::kLarge: + rep_.large.offset = rep_.large.offset + n; + rep_.large.size = rep_.large.size - n; break; - case ByteStringKind::kLarge: { - auto& large = GetLarge(); - const auto large_size = large.size(); - const auto new_large_pos = n; - const auto new_large_size = large_size - n; - large = large.Subcord(new_large_pos, new_large_size); - if (new_large_size <= kSmallByteStringCapacity) { - auto large_copy = std::move(large); - DestroyLarge(); - SetSmall(nullptr, large_copy); - } - } break; } } @@ -440,34 +443,19 @@ void ByteString::RemoveSuffix(size_t n) { } switch (GetKind()) { case ByteStringKind::kSmall: - rep_.small.size -= n; + rep_.small.size = rep_.small.size - n; break; case ByteStringKind::kMedium: - rep_.medium.size -= n; - if (rep_.medium.size <= kSmallByteStringCapacity) { - const auto* refcount = GetMediumReferenceCount(); - SetSmall(GetMediumArena(), GetMedium()); - StrongUnref(refcount); - } + rep_.medium.size = rep_.medium.size - n; + break; + case ByteStringKind::kLarge: + rep_.large.size = rep_.large.size - n; break; - case ByteStringKind::kLarge: { - auto& large = GetLarge(); - const auto large_size = large.size(); - const auto new_large_pos = 0; - const auto new_large_size = large_size - n; - large = large.Subcord(new_large_pos, new_large_size); - if (new_large_size <= kSmallByteStringCapacity) { - auto large_copy = std::move(large); - DestroyLarge(); - SetSmall(nullptr, large_copy); - } - } break; } } void ByteString::CopyToArray(char* absl_nonnull out) const { ABSL_DCHECK(out != nullptr); - switch (GetKind()) { case ByteStringKind::kSmall: { absl::string_view small = GetSmall(); @@ -478,8 +466,8 @@ void ByteString::CopyToArray(char* absl_nonnull out) const { std::memcpy(out, medium.data(), medium.size()); } break; case ByteStringKind::kLarge: { - const absl::Cord& large = GetLarge(); - (CopyCordToArray)(large, out); + (CopyCordToArray)(*rep_.large.data, rep_.large.offset, rep_.large.size, + out); } break; } } @@ -490,14 +478,22 @@ std::string ByteString::ToString() const { return std::string(GetSmall()); case ByteStringKind::kMedium: return std::string(GetMedium()); - case ByteStringKind::kLarge: - return static_cast(GetLarge()); + case ByteStringKind::kLarge: { + std::string result; + absl::StringResizeAndOverwrite( + result, rep_.large.size, + [this](char* buffer, size_t buffer_size) -> size_t { + (CopyCordToArray)(*rep_.large.data, rep_.large.offset, + rep_.large.size, buffer); + return rep_.large.size; + }); + return result; + } } } void ByteString::CopyToString(std::string* absl_nonnull out) const { ABSL_DCHECK(out != nullptr); - switch (GetKind()) { case ByteStringKind::kSmall: out->assign(GetSmall()); @@ -506,14 +502,19 @@ void ByteString::CopyToString(std::string* absl_nonnull out) const { out->assign(GetMedium()); break; case ByteStringKind::kLarge: - absl::CopyCordToString(GetLarge(), out); + absl::StringResizeAndOverwrite( + *out, rep_.large.size, + [this](char* buffer, size_t buffer_size) -> size_t { + (CopyCordToArray)(*rep_.large.data, rep_.large.offset, + rep_.large.size, buffer); + return rep_.large.size; + }); break; } } void ByteString::AppendToString(std::string* absl_nonnull out) const { ABSL_DCHECK(out != nullptr); - switch (GetKind()) { case ByteStringKind::kSmall: out->append(GetSmall()); @@ -522,34 +523,18 @@ void ByteString::AppendToString(std::string* absl_nonnull out) const { out->append(GetMedium()); break; case ByteStringKind::kLarge: - absl::AppendCordToString(GetLarge(), out); + (AppendCordToString)(*rep_.large.data, rep_.large.offset, rep_.large.size, + *out); break; } } -namespace { - -struct ReferenceCountReleaser { - const ReferenceCount* absl_nonnull refcount; - - void operator()() const { StrongUnref(*refcount); } -}; - -} // namespace - absl::Cord ByteString::ToCord() const& { switch (GetKind()) { case ByteStringKind::kSmall: return absl::Cord(GetSmall()); - case ByteStringKind::kMedium: { - const auto* refcount = GetMediumReferenceCount(); - if (refcount != nullptr) { - StrongRef(*refcount); - return absl::MakeCordFromExternal(GetMedium(), - ReferenceCountReleaser{refcount}); - } + case ByteStringKind::kMedium: return absl::Cord(GetMedium()); - } case ByteStringKind::kLarge: return GetLarge(); } @@ -559,16 +544,8 @@ absl::Cord ByteString::ToCord() && { switch (GetKind()) { case ByteStringKind::kSmall: return absl::Cord(GetSmall()); - case ByteStringKind::kMedium: { - const auto* refcount = GetMediumReferenceCount(); - if (refcount != nullptr) { - auto medium = GetMedium(); - SetSmallEmpty(nullptr); - return absl::MakeCordFromExternal(medium, - ReferenceCountReleaser{refcount}); - } + case ByteStringKind::kMedium: return absl::Cord(GetMedium()); - } case ByteStringKind::kLarge: return GetLarge(); } @@ -576,21 +553,13 @@ absl::Cord ByteString::ToCord() && { void ByteString::CopyToCord(absl::Cord* absl_nonnull out) const { ABSL_DCHECK(out != nullptr); - switch (GetKind()) { case ByteStringKind::kSmall: *out = absl::Cord(GetSmall()); break; - case ByteStringKind::kMedium: { - const auto* refcount = GetMediumReferenceCount(); - if (refcount != nullptr) { - StrongRef(*refcount); - *out = absl::MakeCordFromExternal(GetMedium(), - ReferenceCountReleaser{refcount}); - } else { - *out = absl::Cord(GetMedium()); - } - } break; + case ByteStringKind::kMedium: + *out = absl::Cord(GetMedium()); + break; case ByteStringKind::kLarge: *out = GetLarge(); break; @@ -599,21 +568,13 @@ void ByteString::CopyToCord(absl::Cord* absl_nonnull out) const { void ByteString::AppendToCord(absl::Cord* absl_nonnull out) const { ABSL_DCHECK(out != nullptr); - switch (GetKind()) { case ByteStringKind::kSmall: out->Append(GetSmall()); break; - case ByteStringKind::kMedium: { - const auto* refcount = GetMediumReferenceCount(); - if (refcount != nullptr) { - StrongRef(*refcount); - out->Append(absl::MakeCordFromExternal( - GetMedium(), ReferenceCountReleaser{refcount})); - } else { - out->Append(GetMedium()); - } - } break; + case ByteStringKind::kMedium: + out->Append(GetMedium()); + break; case ByteStringKind::kLarge: out->Append(GetLarge()); break; @@ -623,17 +584,22 @@ void ByteString::AppendToCord(absl::Cord* absl_nonnull out) const { absl::string_view ByteString::ToStringView( std::string* absl_nonnull scratch) const { ABSL_DCHECK(scratch != nullptr); - switch (GetKind()) { case ByteStringKind::kSmall: return GetSmall(); case ByteStringKind::kMedium: return GetMedium(); case ByteStringKind::kLarge: - if (auto flat = GetLarge().TryFlat(); flat) { - return *flat; + if (auto flat = rep_.large.data->TryFlat(); flat.has_value()) { + return flat->substr(rep_.large.offset, rep_.large.size); } - absl::CopyCordToString(GetLarge(), scratch); + absl::StringResizeAndOverwrite( + *scratch, rep_.large.size, + [this](char* buffer, size_t buffer_size) -> size_t { + (CopyCordToArray)(*rep_.large.data, rep_.large.offset, + rep_.large.size, buffer); + return rep_.large.size; + }); return absl::string_view(*scratch); } } @@ -652,203 +618,28 @@ absl::string_view ByteString::AsStringView() const { } } -google::protobuf::Arena* absl_nullable ByteString::GetMediumArena( - const MediumByteStringRep& rep) { - if ((rep.owner & kMetadataOwnerBits) == kMetadataOwnerArenaBit) { - return reinterpret_cast(rep.owner & - kMetadataOwnerPointerMask); - } - return nullptr; -} - -const ReferenceCount* absl_nullable ByteString::GetMediumReferenceCount( - const MediumByteStringRep& rep) { - if ((rep.owner & kMetadataOwnerBits) == kMetadataOwnerReferenceCountBit) { - return reinterpret_cast(rep.owner & - kMetadataOwnerPointerMask); - } - return nullptr; -} - -void ByteString::Construct(const ByteString& other, - absl::optional> allocator) { - switch (other.GetKind()) { - case ByteStringKind::kSmall: - rep_.small = other.rep_.small; - if (allocator.has_value()) { - rep_.small.arena = allocator->arena(); - } - break; - case ByteStringKind::kMedium: - if (allocator.has_value() && - allocator->arena() != other.GetMediumArena()) { - SetMedium(allocator->arena(), other.GetMedium()); - } else { - rep_.medium = other.rep_.medium; - StrongRef(GetMediumReferenceCount()); - } - break; - case ByteStringKind::kLarge: - if (allocator.has_value() && allocator->arena() != nullptr) { - SetMedium(allocator->arena(), other.GetLarge()); - } else { - SetLarge(other.GetLarge()); - } - break; - } -} - -void ByteString::Construct(ByteString& other, - absl::optional> allocator) { - switch (other.GetKind()) { - case ByteStringKind::kSmall: - rep_.small = other.rep_.small; - if (allocator.has_value()) { - rep_.small.arena = allocator->arena(); - } - break; - case ByteStringKind::kMedium: - if (allocator.has_value() && - allocator->arena() != other.GetMediumArena()) { - SetMedium(allocator->arena(), other.GetMedium()); - } else { - rep_.medium = other.rep_.medium; - other.rep_.medium.owner = 0; - } - break; - case ByteStringKind::kLarge: - if (allocator.has_value() && allocator->arena() != nullptr) { - SetMedium(allocator->arena(), other.GetLarge()); - } else { - SetLarge(std::move(other.GetLarge())); - } - break; - } -} - -void ByteString::CopyFrom(const ByteString& other) { - ABSL_DCHECK_NE(&other, this); - - switch (other.GetKind()) { - case ByteStringKind::kSmall: - switch (GetKind()) { - case ByteStringKind::kSmall: - break; - case ByteStringKind::kMedium: - DestroyMedium(); - break; - case ByteStringKind::kLarge: - DestroyLarge(); - break; - } - rep_.small = other.rep_.small; - break; - case ByteStringKind::kMedium: - switch (GetKind()) { - case ByteStringKind::kSmall: - rep_.medium = other.rep_.medium; - StrongRef(GetMediumReferenceCount()); - break; - case ByteStringKind::kMedium: - StrongRef(other.GetMediumReferenceCount()); - DestroyMedium(); - rep_.medium = other.rep_.medium; - break; - case ByteStringKind::kLarge: - DestroyLarge(); - rep_.medium = other.rep_.medium; - StrongRef(GetMediumReferenceCount()); - break; - } - break; - case ByteStringKind::kLarge: - switch (GetKind()) { - case ByteStringKind::kSmall: - SetLarge(other.GetLarge()); - break; - case ByteStringKind::kMedium: - DestroyMedium(); - SetLarge(other.GetLarge()); - break; - case ByteStringKind::kLarge: - GetLarge() = other.GetLarge(); - break; - } - break; - } -} - -void ByteString::MoveFrom(ByteString& other) { - ABSL_DCHECK_NE(&other, this); - - switch (other.GetKind()) { - case ByteStringKind::kSmall: - switch (GetKind()) { - case ByteStringKind::kSmall: - break; - case ByteStringKind::kMedium: - DestroyMedium(); - break; - case ByteStringKind::kLarge: - DestroyLarge(); - break; - } - rep_.small = other.rep_.small; - break; - case ByteStringKind::kMedium: - switch (GetKind()) { - case ByteStringKind::kSmall: - rep_.medium = other.rep_.medium; - break; - case ByteStringKind::kMedium: - DestroyMedium(); - rep_.medium = other.rep_.medium; - break; - case ByteStringKind::kLarge: - DestroyLarge(); - rep_.medium = other.rep_.medium; - break; - } - other.rep_.medium.owner = 0; - break; - case ByteStringKind::kLarge: - switch (GetKind()) { - case ByteStringKind::kSmall: - SetLarge(std::move(other.GetLarge())); - break; - case ByteStringKind::kMedium: - DestroyMedium(); - SetLarge(std::move(other.GetLarge())); - break; - case ByteStringKind::kLarge: - GetLarge() = std::move(other.GetLarge()); - break; - } - break; - } -} - ByteString ByteString::Clone(google::protobuf::Arena* absl_nonnull arena) const { ABSL_DCHECK(arena != nullptr); - switch (GetKind()) { - case ByteStringKind::kSmall: - return ByteString(arena, GetSmall()); + case ByteStringKind::kSmall: { + ByteString result(UninitializedTag{}); + result.SetSmall(arena, GetSmall()); + return result; + } case ByteStringKind::kMedium: { google::protobuf::Arena* absl_nullable other_arena = GetMediumArena(); - if (arena != nullptr) { - if (arena == other_arena) { - return *this; - } - return ByteString(arena, GetMedium()); + if (other_arena != arena) { + return From(GetMedium(), arena); } - if (other_arena != nullptr) { - return ByteString(arena, GetMedium()); + return *this; + } + case ByteStringKind::kLarge: { + google::protobuf::Arena* absl_nullable other_arena = GetLargeArena(); + if (other_arena != arena) { + return From(GetLarge(), arena); } return *this; } - case ByteStringKind::kLarge: - return ByteString(arena, GetLarge()); } } @@ -866,80 +657,6 @@ void ByteString::HashValue(absl::HashState state) const { } } -void ByteString::Swap(ByteString& other) { - ABSL_DCHECK_NE(&other, this); - using std::swap; - - switch (other.GetKind()) { - case ByteStringKind::kSmall: - switch (GetKind()) { - case ByteStringKind::kSmall: - // small <=> small - swap(rep_.small, other.rep_.small); - break; - case ByteStringKind::kMedium: - // medium <=> small - swap(rep_, other.rep_); - break; - case ByteStringKind::kLarge: { - absl::Cord cord = std::move(GetLarge()); - DestroyLarge(); - rep_ = other.rep_; - other.SetLarge(std::move(cord)); - } break; - } - break; - case ByteStringKind::kMedium: - switch (GetKind()) { - case ByteStringKind::kSmall: - swap(rep_, other.rep_); - break; - case ByteStringKind::kMedium: - swap(rep_.medium, other.rep_.medium); - break; - case ByteStringKind::kLarge: { - absl::Cord cord = std::move(GetLarge()); - DestroyLarge(); - rep_ = other.rep_; - other.SetLarge(std::move(cord)); - } break; - } - break; - case ByteStringKind::kLarge: - switch (GetKind()) { - case ByteStringKind::kSmall: { - absl::Cord cord = std::move(other.GetLarge()); - other.DestroyLarge(); - other.rep_.small = rep_.small; - SetLarge(std::move(cord)); - } break; - case ByteStringKind::kMedium: { - absl::Cord cord = std::move(other.GetLarge()); - other.DestroyLarge(); - other.rep_.medium = rep_.medium; - SetLarge(std::move(cord)); - } break; - case ByteStringKind::kLarge: - swap(GetLarge(), other.GetLarge()); - break; - } - break; - } -} - -void ByteString::Destroy() { - switch (GetKind()) { - case ByteStringKind::kSmall: - break; - case ByteStringKind::kMedium: - DestroyMedium(); - break; - case ByteStringKind::kLarge: - DestroyLarge(); - break; - } -} - void ByteString::SetSmall(google::protobuf::Arena* absl_nullable arena, absl::string_view string) { ABSL_DCHECK_LE(string.size(), kSmallByteStringCapacity); @@ -962,82 +679,27 @@ void ByteString::SetSmall(google::protobuf::Arena* absl_nullable arena, void ByteString::SetMedium(google::protobuf::Arena* absl_nullable arena, absl::string_view string) { - ABSL_DCHECK_GT(string.size(), kSmallByteStringCapacity); - rep_.header.kind = ByteStringKind::kMedium; - rep_.medium.size = string.size(); - if (arena != nullptr) { - char* data = static_cast( - arena->AllocateAligned(rep_.medium.size, alignof(char))); - std::memcpy(data, string.data(), rep_.medium.size); - rep_.medium.data = data; - rep_.medium.owner = - reinterpret_cast(arena) | kMetadataOwnerArenaBit; - } else { - auto pair = MakeReferenceCountedString(string); - rep_.medium.data = pair.second.data(); - rep_.medium.owner = reinterpret_cast(pair.first) | - kMetadataOwnerReferenceCountBit; - } -} - -void ByteString::SetExternalMedium(absl::string_view string) { - ABSL_DCHECK_GT(string.size(), kSmallByteStringCapacity); rep_.header.kind = ByteStringKind::kMedium; rep_.medium.size = string.size(); rep_.medium.data = string.data(); - rep_.medium.owner = 0; -} - -void ByteString::SetMedium(google::protobuf::Arena* absl_nullable arena, - std::string&& string) { - ABSL_DCHECK_GT(string.size(), kSmallByteStringCapacity); - rep_.header.kind = ByteStringKind::kMedium; - rep_.medium.size = string.size(); - if (arena != nullptr) { - auto* data = google::protobuf::Arena::Create(arena, std::move(string)); - rep_.medium.data = data->data(); - rep_.medium.owner = - reinterpret_cast(arena) | kMetadataOwnerArenaBit; - } else { - auto pair = MakeReferenceCountedString(std::move(string)); - rep_.medium.data = pair.second.data(); - rep_.medium.owner = reinterpret_cast(pair.first) | - kMetadataOwnerReferenceCountBit; - } -} - -void ByteString::SetMedium(google::protobuf::Arena* absl_nonnull arena, - const absl::Cord& cord) { - ABSL_DCHECK_GT(cord.size(), kSmallByteStringCapacity); - rep_.header.kind = ByteStringKind::kMedium; - rep_.medium.size = cord.size(); - char* data = static_cast( - arena->AllocateAligned(rep_.medium.size, alignof(char))); - (CopyCordToArray)(cord, data); - rep_.medium.data = data; - rep_.medium.owner = - reinterpret_cast(arena) | kMetadataOwnerArenaBit; -} - -void ByteString::SetMedium(absl::string_view string, uintptr_t owner) { - ABSL_DCHECK_GT(string.size(), kSmallByteStringCapacity); - ABSL_DCHECK_NE(owner, 0); - rep_.header.kind = ByteStringKind::kMedium; - rep_.medium.size = string.size(); - rep_.medium.data = string.data(); - rep_.medium.owner = owner; -} - -void ByteString::SetLarge(const absl::Cord& cord) { - ABSL_DCHECK_GT(cord.size(), kSmallByteStringCapacity); - rep_.header.kind = ByteStringKind::kLarge; - ::new (static_cast(&rep_.large.data[0])) absl::Cord(cord); + rep_.medium.arena = arena; } -void ByteString::SetLarge(absl::Cord&& cord) { - ABSL_DCHECK_GT(cord.size(), kSmallByteStringCapacity); +void ByteString::SetLarge(google::protobuf::Arena* absl_nullable arena, + const absl::Cord* absl_nonnull cord, size_t offset, + size_t size) { + ABSL_DCHECK_LE(offset, cord->size()); + ABSL_DCHECK_LE(offset, kLargeByteStringMaxSize); rep_.header.kind = ByteStringKind::kLarge; - ::new (static_cast(&rep_.large.data[0])) absl::Cord(std::move(cord)); + rep_.large.offset = offset; + if (size == static_cast(-1)) { + size = cord->size() - offset; + } + ABSL_DCHECK_LE(size, cord->size() - offset); + ABSL_DCHECK_LE(size, kLargeByteStringMaxSize); + rep_.large.size = size; + rep_.large.data = cord; + rep_.large.arena = arena; } absl::string_view LegacyByteString(const ByteString& string, bool stable, diff --git a/common/internal/byte_string.h b/common/internal/byte_string.h index c576e5634..de11f8a9e 100644 --- a/common/internal/byte_string.h +++ b/common/internal/byte_string.h @@ -17,25 +17,19 @@ #include #include -#include +#include #include #include -#include #include #include "absl/base/attributes.h" #include "absl/base/nullability.h" -#include "absl/base/optimization.h" #include "absl/functional/overload.h" #include "absl/hash/hash.h" #include "absl/log/absl_check.h" #include "absl/strings/cord.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" -#include "common/allocator.h" -#include "common/arena.h" -#include "common/internal/reference_count.h" -#include "common/memory.h" #include "google/protobuf/arena.h" namespace cel { @@ -46,15 +40,7 @@ class StringValue; namespace common_internal { -// absl::Cord is trivially relocatable IFF we are not using ASan or MSan. When -// using ASan or MSan absl::Cord will poison/unpoison its inline storage. -#if defined(ABSL_HAVE_ADDRESS_SANITIZER) || defined(ABSL_HAVE_MEMORY_SANITIZER) -#define CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI -#else -#define CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI ABSL_ATTRIBUTE_TRIVIAL_ABI -#endif - -class CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI [[nodiscard]] ByteString; +class [[nodiscard]] ByteString; struct ByteStringTestFriend; @@ -76,11 +62,11 @@ 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 { +struct SmallByteStringRep final { #ifdef _MSC_VER #pragma pack(push, 1) #endif - struct ABSL_ATTRIBUTE_PACKED CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI { + struct ABSL_ATTRIBUTE_PACKED { std::uint8_t kind : 2; std::uint8_t size : 6; }; @@ -98,6 +84,9 @@ inline constexpr size_t kMediumByteStringSizeBits = sizeof(size_t) * 8 - 2; inline constexpr size_t kMediumByteStringMaxSize = (size_t{1} << kMediumByteStringSizeBits) - 1; +inline constexpr size_t kLargeByteStringMaxSize = + (size_t{1} << kMediumByteStringSizeBits) - 1; + inline constexpr size_t kByteStringViewSizeBits = sizeof(size_t) * 8 - 1; inline constexpr size_t kByteStringViewMaxSize = (size_t{1} << kByteStringViewSizeBits) - 1; @@ -105,43 +94,45 @@ inline constexpr size_t kByteStringViewMaxSize = // Representation of medium strings in ByteString. These are either owned by an // arena or managed by a reference count. This is encoded in `owner` following // the same semantics as `cel::Owner`. -struct CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI MediumByteStringRep final { +struct MediumByteStringRep final { #ifdef _MSC_VER #pragma pack(push, 1) #endif - struct ABSL_ATTRIBUTE_PACKED CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI { + struct ABSL_ATTRIBUTE_PACKED { size_t kind : 2; size_t size : kMediumByteStringSizeBits; }; #ifdef _MSC_VER #pragma pack(pop) #endif - const char* data; - uintptr_t owner; + const char* absl_nullability_unknown data; + google::protobuf::Arena* absl_nullable arena; }; // Representation of large strings in ByteString. These are stored as -// `absl::Cord` and never owned by an arena. -struct CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI LargeByteStringRep final { +// a pointer to `absl::Cord`. +struct LargeByteStringRep final { #ifdef _MSC_VER #pragma pack(push, 1) #endif - struct ABSL_ATTRIBUTE_PACKED CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI { + struct ABSL_ATTRIBUTE_PACKED { size_t kind : 2; - size_t padding : kMediumByteStringSizeBits; + size_t offset : kMediumByteStringSizeBits / 2; + size_t size : kMediumByteStringSizeBits / 2; }; #ifdef _MSC_VER #pragma pack(pop) #endif - alignas(absl::Cord) std::byte data[sizeof(absl::Cord)]; + const absl::Cord* absl_nullability_unknown data; + google::protobuf::Arena* absl_nullable arena; }; // Representation of ByteString. -union CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI ByteStringRep final { +union ByteStringRep final { #ifdef _MSC_VER #pragma pack(push, 1) #endif - struct ABSL_ATTRIBUTE_PACKED CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI { + struct ABSL_ATTRIBUTE_PACKED { ByteStringKind kind : 2; } header; #ifdef _MSC_VER @@ -166,91 +157,56 @@ absl::string_view LegacyByteString(const ByteString& string, bool stable, // string is constructed the allocator will not and cannot change. Copying and // moving between different allocators is supported and dealt with // transparently by copying. -class CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI [[nodiscard]] -ByteString final { +class [[nodiscard]] ByteString final { public: - static ByteString Concat(const ByteString& lhs, const ByteString& rhs, - google::protobuf::Arena* absl_nonnull arena); - - ByteString() : ByteString(NewDeleteAllocator()) {} - - explicit ByteString(const char* absl_nullable string) - : ByteString(NewDeleteAllocator(), string) {} - - explicit ByteString(absl::string_view string) - : ByteString(NewDeleteAllocator(), string) {} - - explicit ByteString(const std::string& string) - : ByteString(NewDeleteAllocator(), string) {} - - explicit ByteString(std::string&& string) - : ByteString(NewDeleteAllocator(), std::move(string)) {} - - explicit ByteString(const absl::Cord& cord) - : ByteString(NewDeleteAllocator(), cord) {} - - ByteString(const ByteString& other) noexcept { - Construct(other, /*allocator=*/absl::nullopt); - } - - ByteString(ByteString&& other) noexcept { - Construct(other, /*allocator=*/absl::nullopt); - } - - explicit ByteString(Allocator<> allocator) { - SetSmallEmpty(allocator.arena()); + static ByteString From(const char* absl_nullable value, + google::protobuf::Arena* absl_nonnull arena + ABSL_ATTRIBUTE_LIFETIME_BOUND); + static ByteString From(absl::string_view value, + google::protobuf::Arena* absl_nonnull arena + ABSL_ATTRIBUTE_LIFETIME_BOUND); + static ByteString From(const absl::Cord& value, + google::protobuf::Arena* absl_nonnull arena + ABSL_ATTRIBUTE_LIFETIME_BOUND); + static ByteString From(std::string&& value, + google::protobuf::Arena* absl_nonnull arena + ABSL_ATTRIBUTE_LIFETIME_BOUND); + + static ByteString Wrap(absl::string_view value, + google::protobuf::Arena* absl_nullable arena + ABSL_ATTRIBUTE_LIFETIME_BOUND); + static ByteString Wrap( + const absl::Cord* absl_nonnull value ABSL_ATTRIBUTE_LIFETIME_BOUND, + google::protobuf::Arena* absl_nullable arena ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return Wrap(value, 0, value->size(), arena); } - - ByteString(Allocator<> allocator, const char* absl_nullable string) - : ByteString(allocator, absl::NullSafeStringView(string)) {} - - ByteString(Allocator<> allocator, absl::string_view string); - - ByteString(Allocator<> allocator, const std::string& string); - - ByteString(Allocator<> allocator, std::string&& string); - - ByteString(Allocator<> allocator, const absl::Cord& cord); - - ByteString(Allocator<> allocator, const ByteString& other) { - Construct(other, allocator); - } - - ByteString(Allocator<> allocator, ByteString&& other) { - Construct(other, allocator); + static ByteString Wrap( + const absl::Cord* absl_nonnull value ABSL_ATTRIBUTE_LIFETIME_BOUND, + size_t offset, size_t size, + google::protobuf::Arena* absl_nullable arena ABSL_ATTRIBUTE_LIFETIME_BOUND); + static ByteString Wrap(std::nullptr_t, google::protobuf::Arena*) = delete; + static ByteString Wrap(std::nullptr_t, size_t, size_t, + google::protobuf::Arena*) = delete; + static ByteString Wrap(std::string&& value, google::protobuf::Arena*) = delete; + + static ByteString WrapUnsafe(absl::string_view value); + static ByteString WrapUnsafe(const absl::Cord* absl_nonnull value) { + return WrapUnsafe(value, 0, value->size()); } + static ByteString WrapUnsafe(const absl::Cord* absl_nonnull value, + size_t offset, size_t size); + static ByteString WrapUnsafe(std::nullptr_t) = delete; + static ByteString WrapUnsafe(std::nullptr_t, size_t, size_t) = delete; - ByteString(Borrower borrower, - const char* absl_nullable string ABSL_ATTRIBUTE_LIFETIME_BOUND) - : ByteString(borrower, absl::NullSafeStringView(string)) {} - - ByteString(Borrower borrower, - absl::string_view string ABSL_ATTRIBUTE_LIFETIME_BOUND) - : ByteString(Borrowed(borrower, string)) {} - - ByteString(Borrower borrower, - const absl::Cord& cord ABSL_ATTRIBUTE_LIFETIME_BOUND) - : ByteString(Borrowed(borrower, cord)) {} - - // Creates a medium byte string that is backed by an external string. Should - // only be called from explicit 'Unsafe' factories. - static ByteString FromExternal(absl::string_view string); + static ByteString Concat(const ByteString& lhs, const ByteString& rhs, + google::protobuf::Arena* absl_nonnull arena); - ~ByteString() { Destroy(); } + ByteString() noexcept { SetSmallEmpty(nullptr); } - ByteString& operator=(const ByteString& other) noexcept { - if (ABSL_PREDICT_TRUE(this != &other)) { - CopyFrom(other); - } - return *this; - } - - ByteString& operator=(ByteString&& other) noexcept { - if (ABSL_PREDICT_TRUE(this != &other)) { - MoveFrom(other); - } - return *this; - } + ByteString(const ByteString&) = default; + ByteString(ByteString&&) = default; + ByteString& operator=(const ByteString&) = default; + ByteString& operator=(ByteString&&) = default; bool empty() const; @@ -258,8 +214,6 @@ ByteString final { size_t max_size() const { return kByteStringViewMaxSize; } - absl::string_view Flatten() ABSL_ATTRIBUTE_LIFETIME_BOUND; - absl::optional TryFlat() const ABSL_ATTRIBUTE_LIFETIME_BOUND; @@ -339,10 +293,9 @@ ByteString final { } } - friend void swap(ByteString& lhs, ByteString& rhs) { - if (&lhs != &rhs) { - lhs.Swap(rhs); - } + friend void swap(ByteString& lhs, ByteString& rhs) noexcept { + using std::swap; + swap(lhs.rep_, rhs.rep_); } template @@ -360,21 +313,12 @@ ByteString final { friend absl::string_view LegacyByteString(const ByteString& string, bool stable, google::protobuf::Arena* absl_nonnull arena); - friend struct cel::ArenaTraits; - - struct ExternalStringTag {}; - - static ByteString Borrowed(Borrower borrower, - absl::string_view string - ABSL_ATTRIBUTE_LIFETIME_BOUND); - static ByteString Borrowed( - Borrower borrower, const absl::Cord& cord ABSL_ATTRIBUTE_LIFETIME_BOUND); - - ByteString(const ReferenceCount* absl_nonnull refcount, - absl::string_view string); + struct UninitializedTag { + explicit UninitializedTag() = default; + }; - ByteString(ExternalStringTag, absl::string_view string); + explicit ByteString(UninitializedTag) {} constexpr ByteStringKind GetKind() const { return rep_.header.kind; } @@ -412,39 +356,28 @@ ByteString final { } static google::protobuf::Arena* absl_nullable GetMediumArena( - const MediumByteStringRep& rep); - - const ReferenceCount* absl_nullable GetMediumReferenceCount() const { - ABSL_DCHECK_EQ(GetKind(), ByteStringKind::kMedium); - return GetMediumReferenceCount(rep_.medium); + const MediumByteStringRep& rep) { + return rep.arena; } - static const ReferenceCount* absl_nullable GetMediumReferenceCount( - const MediumByteStringRep& rep); - - uintptr_t GetMediumOwner() const { - ABSL_DCHECK_EQ(GetKind(), ByteStringKind::kMedium); - return rep_.medium.owner; + static absl::Cord GetLarge( + const LargeByteStringRep& rep ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return rep.data->Subcord(rep.offset, rep.size); } - absl::Cord& GetLarge() ABSL_ATTRIBUTE_LIFETIME_BOUND { + absl::Cord GetLarge() const ABSL_ATTRIBUTE_LIFETIME_BOUND { ABSL_DCHECK_EQ(GetKind(), ByteStringKind::kLarge); return GetLarge(rep_.large); } - static absl::Cord& GetLarge( - LargeByteStringRep& rep ABSL_ATTRIBUTE_LIFETIME_BOUND) { - return *std::launder(reinterpret_cast(&rep.data[0])); - } - - const absl::Cord& GetLarge() const ABSL_ATTRIBUTE_LIFETIME_BOUND { + google::protobuf::Arena* absl_nullable GetLargeArena() const { ABSL_DCHECK_EQ(GetKind(), ByteStringKind::kLarge); - return GetLarge(rep_.large); + return GetLargeArena(rep_.large); } - static const absl::Cord& GetLarge( + static google::protobuf::Arena* absl_nullable GetLargeArena( const LargeByteStringRep& rep ABSL_ATTRIBUTE_LIFETIME_BOUND) { - return *std::launder(reinterpret_cast(&rep.data[0])); + return rep.arena; } void SetSmallEmpty(google::protobuf::Arena* absl_nullable arena) { @@ -459,48 +392,14 @@ ByteString final { void SetMedium(google::protobuf::Arena* absl_nullable arena, absl::string_view string); - // This is used to create a medium byte string that is backed by an external - // string. Should only be called from explicit 'Unsafe' factories. - void SetExternalMedium(absl::string_view string); - - void SetMedium(google::protobuf::Arena* absl_nullable arena, std::string&& string); - - void SetMedium(google::protobuf::Arena* absl_nonnull arena, const absl::Cord& cord); - - void SetMedium(absl::string_view string, uintptr_t owner); - - void SetLarge(const absl::Cord& cord); - - void SetLarge(absl::Cord&& cord); - - void Swap(ByteString& other); - - void Construct(const ByteString& other, - absl::optional> allocator); - - void Construct(ByteString& other, absl::optional> allocator); - - void CopyFrom(const ByteString& other); - - void MoveFrom(ByteString& other); - - void Destroy(); - - void DestroyMedium() { - ABSL_DCHECK_EQ(GetKind(), ByteStringKind::kMedium); - DestroyMedium(rep_.medium); + void SetMedium(google::protobuf::Arena* absl_nullable arena, + const std::string* absl_nonnull string) { + SetMedium(arena, absl::string_view(*string)); } - static void DestroyMedium(const MediumByteStringRep& rep) { - StrongUnref(GetMediumReferenceCount(rep)); - } - - void DestroyLarge() { - ABSL_DCHECK_EQ(GetKind(), ByteStringKind::kLarge); - DestroyLarge(rep_.large); - } - - static void DestroyLarge(LargeByteStringRep& rep) { GetLarge(rep).~Cord(); } + void SetLarge(google::protobuf::Arena* absl_nullable arena, + const absl::Cord* absl_nonnull cord, size_t offset = 0, + size_t size = static_cast(-1)); void CopyToArray(char* absl_nonnull out) const; @@ -662,27 +561,8 @@ inline bool operator>=(const absl::Cord& lhs, const ByteString& rhs) { return -rhs.Compare(lhs) >= 0; } -#undef CEL_COMMON_INTERNAL_BYTE_STRING_TRIVIAL_ABI - } // namespace common_internal -template <> -struct ArenaTraits { - using constructible = std::true_type; - - static bool trivially_destructible( - const common_internal::ByteString& byte_string) { - switch (byte_string.GetKind()) { - case common_internal::ByteStringKind::kSmall: - return true; - case common_internal::ByteStringKind::kMedium: - return byte_string.GetMediumReferenceCount() == nullptr; - case common_internal::ByteStringKind::kLarge: - return false; - } - } -}; - } // namespace cel #endif // THIRD_PARTY_CEL_CPP_COMMON_INTERNAL_BYTE_STRING_H_ diff --git a/common/internal/byte_string_test.cc b/common/internal/byte_string_test.cc index c4ae0eae6..b2353ac2d 100644 --- a/common/internal/byte_string_test.cc +++ b/common/internal/byte_string_test.cc @@ -24,10 +24,6 @@ #include "absl/strings/cord.h" #include "absl/strings/cord_test_helpers.h" #include "absl/strings/string_view.h" -#include "absl/types/optional.h" -#include "common/allocator.h" -#include "common/internal/reference_count.h" -#include "common/memory.h" #include "internal/testing.h" #include "google/protobuf/arena.h" @@ -67,17 +63,9 @@ TEST(ByteStringKind, Ostream) { } } -class ByteStringTest : public TestWithParam, - public ByteStringTestFriend { +class ByteStringTest : public ByteStringTestFriend, public ::testing::Test { public: - Allocator<> GetAllocator() { - switch (GetParam()) { - case AllocatorKind::kNewDelete: - return NewDeleteAllocator<>{}; - case AllocatorKind::kArena: - return ArenaAllocator<>(&arena_); - } - } + google::protobuf::Arena* GetArena() { return &arena_; } private: google::protobuf::Arena arena_; @@ -117,686 +105,165 @@ const absl::Cord& GetMediumOrLargeFragmentedCord() { return *medium_or_large; } -TEST_P(ByteStringTest, Default) { - ByteString byte_string = ByteString(GetAllocator(), ""); +TEST_F(ByteStringTest, Default) { + ByteString byte_string = ByteString(); EXPECT_THAT(byte_string, SizeIs(0)); EXPECT_THAT(byte_string, IsEmpty()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); } -TEST_P(ByteStringTest, ConstructNullDataStringView) { - ByteString byte_string(GetAllocator(), absl::string_view()); +TEST_F(ByteStringTest, ConstructNullDataStringView) { + ByteString byte_string = ByteString::From(absl::string_view(), GetArena()); EXPECT_THAT(byte_string, IsEmpty()); - EXPECT_EQ(byte_string.GetArena(), GetAllocator().arena()); + EXPECT_EQ(byte_string.GetArena(), GetArena()); } -TEST_P(ByteStringTest, ConstructSmallCString) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallString().c_str()); +TEST_F(ByteStringTest, ConstructSmallCString) { + ByteString byte_string = + ByteString::From(GetSmallString().c_str(), GetArena()); EXPECT_THAT(byte_string, SizeIs(GetSmallStringView().size())); EXPECT_THAT(byte_string, Not(IsEmpty())); EXPECT_EQ(byte_string, GetSmallStringView()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); - EXPECT_EQ(byte_string.GetArena(), GetAllocator().arena()); + EXPECT_EQ(byte_string.GetArena(), GetArena()); } -TEST_P(ByteStringTest, ConstructMediumCString) { +TEST_F(ByteStringTest, ConstructMediumCString) { ByteString byte_string = - ByteString(GetAllocator(), GetMediumString().c_str()); + ByteString::From(GetMediumString().c_str(), GetArena()); EXPECT_THAT(byte_string, SizeIs(GetMediumStringView().size())); EXPECT_THAT(byte_string, Not(IsEmpty())); EXPECT_EQ(byte_string, GetMediumStringView()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); - EXPECT_EQ(byte_string.GetArena(), GetAllocator().arena()); + EXPECT_EQ(byte_string.GetArena(), GetArena()); } -TEST_P(ByteStringTest, ConstructSmallRValueString) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallString()); +TEST_F(ByteStringTest, ConstructSmallRValueString) { + ByteString byte_string = ByteString::From(GetSmallString(), GetArena()); EXPECT_THAT(byte_string, SizeIs(GetSmallStringView().size())); EXPECT_THAT(byte_string, Not(IsEmpty())); EXPECT_EQ(byte_string, GetSmallStringView()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); - EXPECT_EQ(byte_string.GetArena(), GetAllocator().arena()); + EXPECT_EQ(byte_string.GetArena(), GetArena()); } -TEST_P(ByteStringTest, ConstructSmallLValueString) { - ByteString byte_string = ByteString( - GetAllocator(), static_cast(GetSmallString())); +TEST_F(ByteStringTest, ConstructSmallLValueString) { + ByteString byte_string = ByteString::From( + static_cast(GetSmallString()), GetArena()); EXPECT_THAT(byte_string, SizeIs(GetSmallStringView().size())); EXPECT_THAT(byte_string, Not(IsEmpty())); EXPECT_EQ(byte_string, GetSmallStringView()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); - EXPECT_EQ(byte_string.GetArena(), GetAllocator().arena()); + EXPECT_EQ(byte_string.GetArena(), GetArena()); } -TEST_P(ByteStringTest, ConstructMediumRValueString) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumString()); +TEST_F(ByteStringTest, ConstructMediumRValueString) { + ByteString byte_string = ByteString::From(GetMediumString(), GetArena()); EXPECT_THAT(byte_string, SizeIs(GetMediumStringView().size())); EXPECT_THAT(byte_string, Not(IsEmpty())); EXPECT_EQ(byte_string, GetMediumStringView()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); - EXPECT_EQ(byte_string.GetArena(), GetAllocator().arena()); + EXPECT_EQ(byte_string.GetArena(), GetArena()); } -TEST_P(ByteStringTest, ConstructMediumLValueString) { - ByteString byte_string = ByteString( - GetAllocator(), static_cast(GetMediumString())); +TEST_F(ByteStringTest, ConstructMediumLValueString) { + ByteString byte_string = ByteString::From( + static_cast(GetMediumString()), GetArena()); EXPECT_THAT(byte_string, SizeIs(GetMediumStringView().size())); EXPECT_THAT(byte_string, Not(IsEmpty())); EXPECT_EQ(byte_string, GetMediumStringView()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); - EXPECT_EQ(byte_string.GetArena(), GetAllocator().arena()); + EXPECT_EQ(byte_string.GetArena(), GetArena()); } -TEST_P(ByteStringTest, ConstructSmallCord) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallCord()); +TEST_F(ByteStringTest, ConstructSmallCord) { + ByteString byte_string = ByteString::From(GetSmallCord(), GetArena()); EXPECT_THAT(byte_string, SizeIs(GetSmallStringView().size())); EXPECT_THAT(byte_string, Not(IsEmpty())); EXPECT_EQ(byte_string, GetSmallStringView()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); - EXPECT_EQ(byte_string.GetArena(), GetAllocator().arena()); + EXPECT_EQ(byte_string.GetArena(), GetArena()); } -TEST_P(ByteStringTest, ConstructMediumOrLargeCord) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); +TEST_F(ByteStringTest, ConstructMediumOrLargeCord) { + ByteString byte_string = ByteString::From(GetMediumOrLargeCord(), GetArena()); EXPECT_THAT(byte_string, SizeIs(GetMediumStringView().size())); EXPECT_THAT(byte_string, Not(IsEmpty())); EXPECT_EQ(byte_string, GetMediumStringView()); - if (GetAllocator().arena() == nullptr) { - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kLarge); - } else { - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); - } - EXPECT_EQ(byte_string.GetArena(), GetAllocator().arena()); + EXPECT_EQ(GetKind(byte_string), ByteStringKind::kLarge); + EXPECT_EQ(byte_string.GetArena(), GetArena()); } -TEST(ByteStringTest, BorrowedUnownedString) { -#ifdef NDEBUG - ByteString byte_string = ByteString(Owner::None(), GetMediumStringView()); +TEST_F(ByteStringTest, BorrowedArenaSmallString) { + ByteString byte_string = ByteString::Wrap(GetSmallStringView(), GetArena()); EXPECT_EQ(ByteStringTestFriend::GetKind(byte_string), ByteStringKind::kMedium); - EXPECT_EQ(byte_string.GetArena(), nullptr); - EXPECT_EQ(byte_string, GetMediumStringView()); -#else - EXPECT_DEBUG_DEATH( - static_cast(ByteString(Owner::None(), GetMediumStringView())), - ::testing::_); -#endif -} - -TEST(ByteStringTest, BorrowedUnownedCord) { -#ifdef NDEBUG - ByteString byte_string = ByteString(Owner::None(), GetMediumOrLargeCord()); - EXPECT_EQ(ByteStringTestFriend::GetKind(byte_string), ByteStringKind::kLarge); - EXPECT_EQ(byte_string.GetArena(), nullptr); - EXPECT_EQ(byte_string, GetMediumOrLargeCord()); -#else - EXPECT_DEBUG_DEATH( - static_cast(ByteString(Owner::None(), GetMediumOrLargeCord())), - ::testing::_); -#endif -} - -TEST(ByteStringTest, BorrowedReferenceCountSmallString) { - auto* refcount = new ReferenceCounted(); - Owner owner = Owner::ReferenceCount(refcount); - StrongUnref(refcount); - ByteString byte_string = ByteString(owner, GetSmallStringView()); - EXPECT_EQ(ByteStringTestFriend::GetKind(byte_string), ByteStringKind::kSmall); - EXPECT_EQ(byte_string.GetArena(), nullptr); + EXPECT_EQ(byte_string.GetArena(), GetArena()); EXPECT_EQ(byte_string, GetSmallStringView()); } -TEST(ByteStringTest, BorrowedReferenceCountMediumString) { - auto* refcount = new ReferenceCounted(); - Owner owner = Owner::ReferenceCount(refcount); - StrongUnref(refcount); - ByteString byte_string = ByteString(owner, GetMediumStringView()); +TEST_F(ByteStringTest, BorrowedArenaMediumString) { + ByteString byte_string = ByteString::Wrap(GetMediumStringView(), GetArena()); EXPECT_EQ(ByteStringTestFriend::GetKind(byte_string), ByteStringKind::kMedium); - EXPECT_EQ(byte_string.GetArena(), nullptr); + EXPECT_EQ(byte_string.GetArena(), GetArena()); EXPECT_EQ(byte_string, GetMediumStringView()); } -TEST(ByteStringTest, BorrowedArenaSmallString) { - google::protobuf::Arena arena; - ByteString byte_string = - ByteString(Owner::Arena(&arena), GetSmallStringView()); - EXPECT_EQ(ByteStringTestFriend::GetKind(byte_string), ByteStringKind::kSmall); - EXPECT_EQ(byte_string.GetArena(), &arena); - EXPECT_EQ(byte_string, GetSmallStringView()); -} - -TEST(ByteStringTest, BorrowedArenaMediumString) { - google::protobuf::Arena arena; +TEST_F(ByteStringTest, BorrowedArenaCord) { ByteString byte_string = - ByteString(Owner::Arena(&arena), GetMediumStringView()); - EXPECT_EQ(ByteStringTestFriend::GetKind(byte_string), - ByteStringKind::kMedium); - EXPECT_EQ(byte_string.GetArena(), &arena); - EXPECT_EQ(byte_string, GetMediumStringView()); -} - -TEST(ByteStringTest, BorrowedReferenceCountCord) { - auto* refcount = new ReferenceCounted(); - Owner owner = Owner::ReferenceCount(refcount); - StrongUnref(refcount); - ByteString byte_string = ByteString(owner, GetMediumOrLargeCord()); + ByteString::Wrap(&GetMediumOrLargeCord(), GetArena()); EXPECT_EQ(ByteStringTestFriend::GetKind(byte_string), ByteStringKind::kLarge); - EXPECT_EQ(byte_string.GetArena(), nullptr); - EXPECT_EQ(byte_string, GetMediumOrLargeCord()); -} - -TEST(ByteStringTest, BorrowedArenaCord) { - google::protobuf::Arena arena; - Owner owner = Owner::Arena(&arena); - ByteString byte_string = ByteString(owner, GetMediumOrLargeCord()); - EXPECT_EQ(ByteStringTestFriend::GetKind(byte_string), - ByteStringKind::kMedium); - EXPECT_EQ(byte_string.GetArena(), &arena); + EXPECT_EQ(byte_string.GetArena(), GetArena()); EXPECT_EQ(byte_string, GetMediumOrLargeCord()); } -TEST_P(ByteStringTest, CopyConstruct) { - ByteString small_byte_string = - ByteString(GetAllocator(), GetSmallStringView()); - ByteString medium_byte_string = - ByteString(GetAllocator(), GetMediumStringView()); - ByteString large_byte_string = - ByteString(GetAllocator(), GetMediumOrLargeCord()); - - EXPECT_EQ(ByteString(NewDeleteAllocator(), small_byte_string), - small_byte_string); - EXPECT_EQ(ByteString(NewDeleteAllocator(), medium_byte_string), - medium_byte_string); - EXPECT_EQ(ByteString(NewDeleteAllocator(), large_byte_string), - large_byte_string); - - google::protobuf::Arena arena; - EXPECT_EQ(ByteString(ArenaAllocator(&arena), small_byte_string), - small_byte_string); - EXPECT_EQ(ByteString(ArenaAllocator(&arena), medium_byte_string), - medium_byte_string); - EXPECT_EQ(ByteString(ArenaAllocator(&arena), large_byte_string), - large_byte_string); - - EXPECT_EQ(ByteString(GetAllocator(), small_byte_string), small_byte_string); - EXPECT_EQ(ByteString(GetAllocator(), medium_byte_string), medium_byte_string); - EXPECT_EQ(ByteString(GetAllocator(), large_byte_string), large_byte_string); - - EXPECT_EQ(ByteString(small_byte_string), small_byte_string); - EXPECT_EQ(ByteString(medium_byte_string), medium_byte_string); - EXPECT_EQ(ByteString(large_byte_string), large_byte_string); -} - -TEST_P(ByteStringTest, CopyConstructFromExternal) { - ByteString small_byte_string = ByteString::FromExternal(GetSmallStringView()); - ByteString medium_byte_string = - ByteString::FromExternal(GetMediumStringView()); - - EXPECT_EQ(ByteString(NewDeleteAllocator(), small_byte_string), - small_byte_string); - EXPECT_EQ(ByteString(NewDeleteAllocator(), medium_byte_string), - medium_byte_string); - - google::protobuf::Arena arena; - EXPECT_EQ(ByteString(ArenaAllocator(&arena), small_byte_string), - small_byte_string); - EXPECT_EQ(ByteString(ArenaAllocator(&arena), medium_byte_string), - medium_byte_string); - - EXPECT_EQ(ByteString(GetAllocator(), small_byte_string), small_byte_string); - EXPECT_EQ(ByteString(GetAllocator(), medium_byte_string), medium_byte_string); - - EXPECT_EQ(ByteString(small_byte_string), small_byte_string); - EXPECT_EQ(ByteString(medium_byte_string), medium_byte_string); -} - -TEST_P(ByteStringTest, MoveConstruct) { - const auto& small_byte_string = [this]() { - return ByteString(GetAllocator(), GetSmallStringView()); - }; - const auto& medium_byte_string = [this]() { - return ByteString(GetAllocator(), GetMediumStringView()); - }; - const auto& large_byte_string = [this]() { - return ByteString(GetAllocator(), GetMediumOrLargeCord()); - }; - - EXPECT_EQ(ByteString(NewDeleteAllocator(), small_byte_string()), - small_byte_string()); - EXPECT_EQ(ByteString(NewDeleteAllocator(), medium_byte_string()), - medium_byte_string()); - EXPECT_EQ(ByteString(NewDeleteAllocator(), large_byte_string()), - large_byte_string()); - - google::protobuf::Arena arena; - EXPECT_EQ(ByteString(ArenaAllocator(&arena), small_byte_string()), - small_byte_string()); - EXPECT_EQ(ByteString(ArenaAllocator(&arena), medium_byte_string()), - medium_byte_string()); - EXPECT_EQ(ByteString(ArenaAllocator(&arena), large_byte_string()), - large_byte_string()); - - EXPECT_EQ(ByteString(GetAllocator(), small_byte_string()), - small_byte_string()); - EXPECT_EQ(ByteString(GetAllocator(), medium_byte_string()), - medium_byte_string()); - EXPECT_EQ(ByteString(GetAllocator(), large_byte_string()), - large_byte_string()); - - EXPECT_EQ(ByteString(small_byte_string()), small_byte_string()); - EXPECT_EQ(ByteString(medium_byte_string()), medium_byte_string()); - EXPECT_EQ(ByteString(large_byte_string()), large_byte_string()); -} - -TEST_P(ByteStringTest, MoveConstructFromExternal) { - const auto& small_byte_string = []() { - return ByteString::FromExternal(GetSmallStringView()); - }; - const auto& medium_byte_string = []() { - return ByteString::FromExternal(GetMediumStringView()); - }; - - EXPECT_EQ(ByteString(NewDeleteAllocator(), small_byte_string()), - small_byte_string()); - EXPECT_EQ(ByteString(NewDeleteAllocator(), medium_byte_string()), - medium_byte_string()); - - google::protobuf::Arena arena; - EXPECT_EQ(ByteString(ArenaAllocator(&arena), small_byte_string()), - small_byte_string()); - EXPECT_EQ(ByteString(ArenaAllocator(&arena), medium_byte_string()), - medium_byte_string()); - - EXPECT_EQ(ByteString(GetAllocator(), small_byte_string()), - small_byte_string()); - EXPECT_EQ(ByteString(GetAllocator(), medium_byte_string()), - medium_byte_string()); - - EXPECT_EQ(ByteString(small_byte_string()), small_byte_string()); - EXPECT_EQ(ByteString(medium_byte_string()), medium_byte_string()); -} - -TEST_P(ByteStringTest, CopyFromByteString) { - ByteString small_byte_string = - ByteString(GetAllocator(), GetSmallStringView()); - ByteString medium_byte_string = - ByteString(GetAllocator(), GetMediumStringView()); - ByteString large_byte_string = - ByteString(GetAllocator(), GetMediumOrLargeCord()); - - ByteString new_delete_byte_string(NewDeleteAllocator<>{}); - // Small <= Small - new_delete_byte_string = small_byte_string; - EXPECT_EQ(new_delete_byte_string, small_byte_string); - // Small <= Medium - new_delete_byte_string = medium_byte_string; - EXPECT_EQ(new_delete_byte_string, medium_byte_string); - // Medium <= Medium - new_delete_byte_string = medium_byte_string; - EXPECT_EQ(new_delete_byte_string, medium_byte_string); - // Medium <= Large - new_delete_byte_string = large_byte_string; - EXPECT_EQ(new_delete_byte_string, large_byte_string); - // Large <= Large - new_delete_byte_string = large_byte_string; - EXPECT_EQ(new_delete_byte_string, large_byte_string); - // Large <= Small - new_delete_byte_string = small_byte_string; - EXPECT_EQ(new_delete_byte_string, small_byte_string); - // Small <= Large - new_delete_byte_string = large_byte_string; - EXPECT_EQ(new_delete_byte_string, large_byte_string); - // Large <= Medium - new_delete_byte_string = medium_byte_string; - EXPECT_EQ(new_delete_byte_string, medium_byte_string); - // Medium <= Small - new_delete_byte_string = small_byte_string; - EXPECT_EQ(new_delete_byte_string, small_byte_string); - - google::protobuf::Arena arena; - ByteString arena_byte_string(ArenaAllocator<>{&arena}); - // Small <= Small - arena_byte_string = small_byte_string; - EXPECT_EQ(arena_byte_string, small_byte_string); - // Small <= Medium - arena_byte_string = medium_byte_string; - EXPECT_EQ(arena_byte_string, medium_byte_string); - // Medium <= Medium - arena_byte_string = medium_byte_string; - EXPECT_EQ(arena_byte_string, medium_byte_string); - // Medium <= Large - arena_byte_string = large_byte_string; - EXPECT_EQ(arena_byte_string, large_byte_string); - // Large <= Large - arena_byte_string = large_byte_string; - EXPECT_EQ(arena_byte_string, large_byte_string); - // Large <= Small - arena_byte_string = small_byte_string; - EXPECT_EQ(arena_byte_string, small_byte_string); - // Small <= Large - arena_byte_string = large_byte_string; - EXPECT_EQ(arena_byte_string, large_byte_string); - // Large <= Medium - arena_byte_string = medium_byte_string; - EXPECT_EQ(arena_byte_string, medium_byte_string); - // Medium <= Small - arena_byte_string = small_byte_string; - EXPECT_EQ(arena_byte_string, small_byte_string); - - ByteString allocator_byte_string(GetAllocator()); - // Small <= Small - allocator_byte_string = small_byte_string; - EXPECT_EQ(allocator_byte_string, small_byte_string); - // Small <= Medium - allocator_byte_string = medium_byte_string; - EXPECT_EQ(allocator_byte_string, medium_byte_string); - // Medium <= Medium - allocator_byte_string = medium_byte_string; - EXPECT_EQ(allocator_byte_string, medium_byte_string); - // Medium <= Large - allocator_byte_string = large_byte_string; - EXPECT_EQ(allocator_byte_string, large_byte_string); - // Large <= Large - allocator_byte_string = large_byte_string; - EXPECT_EQ(allocator_byte_string, large_byte_string); - // Large <= Small - allocator_byte_string = small_byte_string; - EXPECT_EQ(allocator_byte_string, small_byte_string); - // Small <= Large - allocator_byte_string = large_byte_string; - EXPECT_EQ(allocator_byte_string, large_byte_string); - // Large <= Medium - allocator_byte_string = medium_byte_string; - EXPECT_EQ(allocator_byte_string, medium_byte_string); - // Medium <= Small - allocator_byte_string = small_byte_string; - EXPECT_EQ(allocator_byte_string, small_byte_string); - - // Miscellaneous cases not covered above. - // Large <= Medium Arena String - ByteString large_new_delete_byte_string(NewDeleteAllocator<>{}, - GetMediumOrLargeCord()); - ByteString medium_arena_byte_string(ArenaAllocator<>{&arena}, - GetMediumStringView()); - large_new_delete_byte_string = medium_arena_byte_string; - EXPECT_EQ(large_new_delete_byte_string, medium_arena_byte_string); -} - -TEST_P(ByteStringTest, MoveFrom) { - const auto& small_byte_string = [this]() { - return ByteString(GetAllocator(), GetSmallStringView()); - }; - const auto& medium_byte_string = [this]() { - return ByteString(GetAllocator(), GetMediumStringView()); - }; - const auto& large_byte_string = [this]() { - return ByteString(GetAllocator(), GetMediumOrLargeCord()); - }; - - ByteString new_delete_byte_string(NewDeleteAllocator<>{}); - // Small <= Small - new_delete_byte_string = small_byte_string(); - EXPECT_EQ(new_delete_byte_string, small_byte_string()); - // Small <= Medium - new_delete_byte_string = medium_byte_string(); - EXPECT_EQ(new_delete_byte_string, medium_byte_string()); - // Medium <= Medium - new_delete_byte_string = medium_byte_string(); - EXPECT_EQ(new_delete_byte_string, medium_byte_string()); - // Medium <= Large - new_delete_byte_string = large_byte_string(); - EXPECT_EQ(new_delete_byte_string, large_byte_string()); - // Large <= Large - new_delete_byte_string = large_byte_string(); - EXPECT_EQ(new_delete_byte_string, large_byte_string()); - // Large <= Small - new_delete_byte_string = small_byte_string(); - EXPECT_EQ(new_delete_byte_string, small_byte_string()); - // Small <= Large - new_delete_byte_string = large_byte_string(); - EXPECT_EQ(new_delete_byte_string, large_byte_string()); - // Large <= Medium - new_delete_byte_string = medium_byte_string(); - EXPECT_EQ(new_delete_byte_string, medium_byte_string()); - // Medium <= Small - new_delete_byte_string = small_byte_string(); - EXPECT_EQ(new_delete_byte_string, small_byte_string()); - - google::protobuf::Arena arena; - ByteString arena_byte_string(ArenaAllocator<>{&arena}); - // Small <= Small - arena_byte_string = small_byte_string(); - EXPECT_EQ(arena_byte_string, small_byte_string()); - // Small <= Medium - arena_byte_string = medium_byte_string(); - EXPECT_EQ(arena_byte_string, medium_byte_string()); - // Medium <= Medium - arena_byte_string = medium_byte_string(); - EXPECT_EQ(arena_byte_string, medium_byte_string()); - // Medium <= Large - arena_byte_string = large_byte_string(); - EXPECT_EQ(arena_byte_string, large_byte_string()); - // Large <= Large - arena_byte_string = large_byte_string(); - EXPECT_EQ(arena_byte_string, large_byte_string()); - // Large <= Small - arena_byte_string = small_byte_string(); - EXPECT_EQ(arena_byte_string, small_byte_string()); - // Small <= Large - arena_byte_string = large_byte_string(); - EXPECT_EQ(arena_byte_string, large_byte_string()); - // Large <= Medium - arena_byte_string = medium_byte_string(); - EXPECT_EQ(arena_byte_string, medium_byte_string()); - // Medium <= Small - arena_byte_string = small_byte_string(); - EXPECT_EQ(arena_byte_string, small_byte_string()); - - ByteString allocator_byte_string(GetAllocator()); - // Small <= Small - allocator_byte_string = small_byte_string(); - EXPECT_EQ(allocator_byte_string, small_byte_string()); - // Small <= Medium - allocator_byte_string = medium_byte_string(); - EXPECT_EQ(allocator_byte_string, medium_byte_string()); - // Medium <= Medium - allocator_byte_string = medium_byte_string(); - EXPECT_EQ(allocator_byte_string, medium_byte_string()); - // Medium <= Large - allocator_byte_string = large_byte_string(); - EXPECT_EQ(allocator_byte_string, large_byte_string()); - // Large <= Large - allocator_byte_string = large_byte_string(); - EXPECT_EQ(allocator_byte_string, large_byte_string()); - // Large <= Small - allocator_byte_string = small_byte_string(); - EXPECT_EQ(allocator_byte_string, small_byte_string()); - // Small <= Large - allocator_byte_string = large_byte_string(); - EXPECT_EQ(allocator_byte_string, large_byte_string()); - // Large <= Medium - allocator_byte_string = medium_byte_string(); - EXPECT_EQ(allocator_byte_string, medium_byte_string()); - // Medium <= Small - allocator_byte_string = small_byte_string(); - EXPECT_EQ(allocator_byte_string, small_byte_string()); - - // Miscellaneous cases not covered above. - // Large <= Medium Arena String - ByteString large_new_delete_byte_string(NewDeleteAllocator<>{}, - GetMediumOrLargeCord()); - ByteString medium_arena_byte_string(ArenaAllocator<>{&arena}, - GetMediumStringView()); - large_new_delete_byte_string = std::move(medium_arena_byte_string); - EXPECT_EQ(large_new_delete_byte_string, GetMediumStringView()); -} - -TEST_P(ByteStringTest, Swap) { - using std::swap; - ByteString empty_byte_string(GetAllocator()); - ByteString small_byte_string = - ByteString(GetAllocator(), GetSmallStringView()); - ByteString medium_byte_string = - ByteString(GetAllocator(), GetMediumStringView()); - ByteString large_byte_string = - ByteString(GetAllocator(), GetMediumOrLargeCord()); - - // Small <=> Small - swap(empty_byte_string, small_byte_string); - EXPECT_EQ(empty_byte_string, GetSmallStringView()); - EXPECT_EQ(small_byte_string, ""); - swap(empty_byte_string, small_byte_string); - EXPECT_EQ(empty_byte_string, ""); - EXPECT_EQ(small_byte_string, GetSmallStringView()); - - // Small <=> Medium - swap(small_byte_string, medium_byte_string); - EXPECT_EQ(small_byte_string, GetMediumStringView()); - EXPECT_EQ(medium_byte_string, GetSmallStringView()); - swap(small_byte_string, medium_byte_string); - EXPECT_EQ(small_byte_string, GetSmallStringView()); - EXPECT_EQ(medium_byte_string, GetMediumStringView()); - - // Small <=> Large - swap(small_byte_string, large_byte_string); - EXPECT_EQ(small_byte_string, GetMediumOrLargeCord()); - EXPECT_EQ(large_byte_string, GetSmallStringView()); - swap(small_byte_string, large_byte_string); - EXPECT_EQ(small_byte_string, GetSmallStringView()); - EXPECT_EQ(large_byte_string, GetMediumOrLargeCord()); - - // Medium <=> Medium - static constexpr absl::string_view kDifferentMediumStringView = - "A different string that is too large for the small string optimization!"; - ByteString other_medium_byte_string = - ByteString(GetAllocator(), kDifferentMediumStringView); - swap(medium_byte_string, other_medium_byte_string); - EXPECT_EQ(medium_byte_string, kDifferentMediumStringView); - EXPECT_EQ(other_medium_byte_string, GetMediumStringView()); - swap(medium_byte_string, other_medium_byte_string); - EXPECT_EQ(medium_byte_string, GetMediumStringView()); - EXPECT_EQ(other_medium_byte_string, kDifferentMediumStringView); - - // Medium <=> Large - swap(medium_byte_string, large_byte_string); - EXPECT_EQ(medium_byte_string, GetMediumOrLargeCord()); - EXPECT_EQ(large_byte_string, GetMediumStringView()); - swap(medium_byte_string, large_byte_string); - EXPECT_EQ(medium_byte_string, GetMediumStringView()); - EXPECT_EQ(large_byte_string, GetMediumOrLargeCord()); - - // Large <=> Large - const absl::Cord different_medium_or_large_cord = - absl::Cord(kDifferentMediumStringView); - ByteString other_large_byte_string = - ByteString(GetAllocator(), different_medium_or_large_cord); - swap(large_byte_string, other_large_byte_string); - EXPECT_EQ(large_byte_string, different_medium_or_large_cord); - EXPECT_EQ(other_large_byte_string, GetMediumStringView()); - swap(large_byte_string, other_large_byte_string); - EXPECT_EQ(large_byte_string, GetMediumStringView()); - EXPECT_EQ(other_large_byte_string, different_medium_or_large_cord); - - // Miscellaneous cases not covered above. These do not swap a second time to - // restore state, so they are destructive. - // Small <=> Different Allocator Medium - ByteString medium_new_delete_byte_string = - ByteString(NewDeleteAllocator<>{}, kDifferentMediumStringView); - swap(empty_byte_string, medium_new_delete_byte_string); - EXPECT_EQ(empty_byte_string, kDifferentMediumStringView); - EXPECT_EQ(medium_new_delete_byte_string, ""); - // Small <=> Different Allocator Large - ByteString large_new_delete_byte_string = - ByteString(NewDeleteAllocator<>{}, GetMediumOrLargeCord()); - swap(small_byte_string, large_new_delete_byte_string); - EXPECT_EQ(small_byte_string, GetMediumOrLargeCord()); - EXPECT_EQ(large_new_delete_byte_string, GetSmallStringView()); - // Medium <=> Different Allocator Large - large_new_delete_byte_string = - ByteString(NewDeleteAllocator<>{}, different_medium_or_large_cord); - swap(medium_byte_string, large_new_delete_byte_string); - EXPECT_EQ(medium_byte_string, different_medium_or_large_cord); - EXPECT_EQ(large_new_delete_byte_string, GetMediumStringView()); - // Medium <=> Different Allocator Medium - medium_byte_string = ByteString(GetAllocator(), GetMediumStringView()); - medium_new_delete_byte_string = - ByteString(NewDeleteAllocator<>{}, kDifferentMediumStringView); - swap(medium_byte_string, medium_new_delete_byte_string); - EXPECT_EQ(medium_byte_string, kDifferentMediumStringView); - EXPECT_EQ(medium_new_delete_byte_string, GetMediumStringView()); -} - -TEST_P(ByteStringTest, FlattenSmall) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); - EXPECT_EQ(byte_string.Flatten(), GetSmallStringView()); - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); -} - -TEST_P(ByteStringTest, FlattenMedium) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); - EXPECT_EQ(byte_string.Flatten(), GetMediumStringView()); - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); -} - -TEST_P(ByteStringTest, FlattenLarge) { - if (GetAllocator().arena() != nullptr) { - GTEST_SKIP(); - } - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kLarge); - EXPECT_EQ(byte_string.Flatten(), GetMediumStringView()); - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kLarge); -} - -TEST_P(ByteStringTest, TryFlatSmall) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); +TEST_F(ByteStringTest, TryFlatSmall) { + ByteString byte_string = ByteString::From(GetSmallStringView(), GetArena()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); EXPECT_THAT(byte_string.TryFlat(), Optional(GetSmallStringView())); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); } -TEST_P(ByteStringTest, TryFlatMedium) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); +TEST_F(ByteStringTest, TryFlatMedium) { + ByteString byte_string = ByteString::From(GetMediumStringView(), GetArena()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); EXPECT_THAT(byte_string.TryFlat(), Optional(GetMediumStringView())); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); } -TEST_P(ByteStringTest, TryFlatLarge) { - if (GetAllocator().arena() != nullptr) { +TEST_F(ByteStringTest, TryFlatLarge) { + if (GetArena() != nullptr) { GTEST_SKIP(); } ByteString byte_string = - ByteString(GetAllocator(), GetMediumOrLargeFragmentedCord()); + ByteString::From(GetMediumOrLargeFragmentedCord(), GetArena()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kLarge); EXPECT_THAT(byte_string.TryFlat(), Eq(std::nullopt)); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kLarge); } -TEST_P(ByteStringTest, Equals) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); +TEST_F(ByteStringTest, Equals) { + ByteString byte_string = ByteString::From(GetMediumOrLargeCord(), GetArena()); EXPECT_TRUE(byte_string.Equals(GetMediumStringView())); } -TEST_P(ByteStringTest, Compare) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); +TEST_F(ByteStringTest, Compare) { + ByteString byte_string = ByteString::From(GetMediumOrLargeCord(), GetArena()); EXPECT_EQ(byte_string.Compare(GetMediumStringView()), 0); EXPECT_EQ(byte_string.Compare(GetMediumOrLargeCord()), 0); } -TEST_P(ByteStringTest, StartsWith) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); +TEST_F(ByteStringTest, StartsWith) { + ByteString byte_string = ByteString::From(GetMediumOrLargeCord(), GetArena()); EXPECT_TRUE(byte_string.StartsWith( GetMediumStringView().substr(0, kSmallByteStringCapacity))); EXPECT_TRUE(byte_string.StartsWith( GetMediumOrLargeCord().Subcord(0, kSmallByteStringCapacity))); } -TEST_P(ByteStringTest, EndsWith) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); +TEST_F(ByteStringTest, EndsWith) { + ByteString byte_string = ByteString::From(GetMediumOrLargeCord(), GetArena()); EXPECT_TRUE(byte_string.EndsWith( GetMediumStringView().substr(kSmallByteStringCapacity))); EXPECT_TRUE(byte_string.EndsWith(GetMediumOrLargeCord().Subcord( @@ -804,8 +271,8 @@ TEST_P(ByteStringTest, EndsWith) { GetMediumOrLargeCord().size() - kSmallByteStringCapacity))); } -TEST_P(ByteStringTest, Find) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); +TEST_F(ByteStringTest, Find) { + ByteString byte_string = ByteString::From(GetMediumStringView(), GetArena()); // Find string_view EXPECT_THAT(byte_string.Find("A string"), Optional(0)); @@ -833,14 +300,14 @@ TEST_P(ByteStringTest, Find) { EXPECT_THAT(byte_string.Find(absl::Cord(""), 3), Optional(3)); } -TEST_P(ByteStringTest, FindEdgeCases) { - ByteString empty_byte_string(GetAllocator(), ""); +TEST_F(ByteStringTest, FindEdgeCases) { + ByteString empty_byte_string; EXPECT_THAT(empty_byte_string.Find("a"), Eq(std::nullopt)); EXPECT_THAT(empty_byte_string.Find(""), Optional(0)); ByteString cord_byte_string = - ByteString(GetAllocator(), GetMediumOrLargeCord()); + ByteString::From(GetMediumOrLargeCord(), GetArena()); EXPECT_THAT(cord_byte_string.Find("not found"), Eq(std::nullopt)); - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); + ByteString byte_string = ByteString::From(GetMediumStringView(), GetArena()); // Needle longer than haystack. EXPECT_THAT(byte_string.Find(std::string(byte_string.size() + 1, 'a')), @@ -875,13 +342,13 @@ TEST_P(ByteStringTest, FindEdgeCases) { // Search with fragmented cord needle on string_view backed ByteString with // partial match. - ByteString partial_match_haystack(GetAllocator(), "abababac"); + ByteString partial_match_haystack = ByteString::WrapUnsafe("abababac"); absl::Cord partial_match_needle = absl::MakeFragmentedCord({"aba", "c"}); EXPECT_THAT(partial_match_haystack.Find(partial_match_needle), Optional(4)); // Search with fragmented cord needle where first chunk is found but not // enough space for the rest. - ByteString short_haystack(GetAllocator(), "abcdefg"); + ByteString short_haystack = ByteString::WrapUnsafe("abcdefg"); absl::Cord needle_too_long = absl::MakeFragmentedCord({"ef", "gh"}); EXPECT_THAT(short_haystack.Find(needle_too_long), Eq(std::nullopt)); @@ -891,8 +358,8 @@ TEST_P(ByteStringTest, FindEdgeCases) { EXPECT_THAT(byte_string.Find(fragmented_empty_cord, 3), Optional(3)); // Search for suffix in a fragmented cord. - ByteString fragmented_cord_byte_string(GetAllocator(), - GetMediumOrLargeFragmentedCord()); + ByteString fragmented_cord_byte_string = + ByteString::From(GetMediumOrLargeFragmentedCord(), GetArena()); EXPECT_THAT(fragmented_cord_byte_string.Find(suffix_sv), Optional(fragmented_cord_byte_string.size() - suffix_sv.size())); EXPECT_THAT(fragmented_cord_byte_string.Find(absl::Cord(suffix_sv)), @@ -900,311 +367,305 @@ TEST_P(ByteStringTest, FindEdgeCases) { } #ifndef NDEBUG -TEST_P(ByteStringTest, FindOutOfBounds) { - ByteString byte_string = ByteString(GetAllocator(), "test"); +TEST_F(ByteStringTest, FindOutOfBounds) { + ByteString byte_string = ByteString::WrapUnsafe("test"); EXPECT_DEATH(byte_string.Find("t", 5), _); } #endif -TEST_P(ByteStringTest, Substring) { +TEST_F(ByteStringTest, Substring) { // small byte_string substring - ByteString small_byte_string = - ByteString(GetAllocator(), GetSmallStringView()); + ByteString small_byte_string = ByteString::WrapUnsafe(GetSmallStringView()); EXPECT_EQ(small_byte_string.Substring(1, 5), GetSmallStringView().substr(1, 4)); EXPECT_EQ(small_byte_string.Substring(0, small_byte_string.size()), GetSmallStringView()); EXPECT_EQ(small_byte_string.Substring(1, 1), ""); // medium byte_string substring - ByteString medium_byte_string = - ByteString(GetAllocator(), GetMediumStringView()); + ByteString medium_byte_string = ByteString::WrapUnsafe(GetMediumStringView()); EXPECT_EQ(medium_byte_string.Substring(2, 12), GetMediumStringView().substr(2, 10)); EXPECT_EQ(medium_byte_string.Substring(0, medium_byte_string.size()), GetMediumStringView()); // large byte_string substring ByteString large_byte_string = - ByteString(GetAllocator(), GetMediumOrLargeCord()); + ByteString::WrapUnsafe(&GetMediumOrLargeCord()); EXPECT_EQ(large_byte_string.Substring(3, 15), GetMediumOrLargeCord().Subcord(3, 12)); EXPECT_EQ(large_byte_string.Substring(0, large_byte_string.size()), GetMediumOrLargeCord()); // substring with one parameter - ByteString tacocat_byte_string = ByteString(GetAllocator(), "tacocat"); + ByteString tacocat_byte_string = ByteString::WrapUnsafe("tacocat"); EXPECT_EQ(tacocat_byte_string.Substring(4), "cat"); } -TEST_P(ByteStringTest, SubstringEdgeCases) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); +TEST_F(ByteStringTest, SubstringEdgeCases) { + ByteString byte_string = ByteString::WrapUnsafe(GetSmallStringView()); EXPECT_EQ(byte_string.Substring(byte_string.size(), byte_string.size()), ""); EXPECT_EQ(byte_string.Substring(0, 0), ""); } #ifndef NDEBUG -TEST_P(ByteStringTest, SubstringOutOfBounds) { - ByteString byte_string = ByteString(GetAllocator(), "test"); +TEST_F(ByteStringTest, SubstringOutOfBounds) { + ByteString byte_string = ByteString::WrapUnsafe("test"); EXPECT_DEATH(static_cast(byte_string.Substring(5, 5)), _); EXPECT_DEATH(static_cast(byte_string.Substring(0, 5)), _); EXPECT_DEATH(static_cast(byte_string.Substring(3, 2)), _); } #endif -TEST_P(ByteStringTest, RemovePrefixSmall) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); +TEST_F(ByteStringTest, RemovePrefixSmall) { + ByteString byte_string = ByteString::WrapUnsafe(GetSmallStringView()); byte_string.RemovePrefix(1); EXPECT_EQ(byte_string, GetSmallStringView().substr(1)); } -TEST_P(ByteStringTest, RemovePrefixMedium) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); +TEST_F(ByteStringTest, RemovePrefixMedium) { + ByteString byte_string = ByteString::WrapUnsafe(GetMediumStringView()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); byte_string.RemovePrefix(byte_string.size() - kSmallByteStringCapacity); - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); + EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); EXPECT_EQ(byte_string, GetMediumStringView().substr(GetMediumStringView().size() - kSmallByteStringCapacity)); } -TEST_P(ByteStringTest, RemovePrefixMediumOrLarge) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); +TEST_F(ByteStringTest, RemovePrefixMediumOrLarge) { + ByteString byte_string = ByteString::WrapUnsafe(&GetMediumOrLargeCord()); byte_string.RemovePrefix(byte_string.size() - kSmallByteStringCapacity); - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); + EXPECT_EQ(GetKind(byte_string), ByteStringKind::kLarge); EXPECT_EQ(byte_string, GetMediumStringView().substr(GetMediumStringView().size() - kSmallByteStringCapacity)); } -TEST_P(ByteStringTest, RemoveSuffixSmall) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); +TEST_F(ByteStringTest, RemoveSuffixSmall) { + ByteString byte_string = ByteString::WrapUnsafe(GetSmallStringView()); byte_string.RemoveSuffix(1); EXPECT_EQ(byte_string, GetSmallStringView().substr(0, GetSmallStringView().size() - 1)); } -TEST_P(ByteStringTest, RemoveSuffixMedium) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); +TEST_F(ByteStringTest, RemoveSuffixMedium) { + ByteString byte_string = ByteString::WrapUnsafe(GetMediumStringView()); EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); byte_string.RemoveSuffix(byte_string.size() - kSmallByteStringCapacity); - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); + EXPECT_EQ(GetKind(byte_string), ByteStringKind::kMedium); EXPECT_EQ(byte_string, GetMediumStringView().substr(0, kSmallByteStringCapacity)); } -TEST_P(ByteStringTest, RemoveSuffixMediumOrLarge) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); +TEST_F(ByteStringTest, RemoveSuffixMediumOrLarge) { + ByteString byte_string = ByteString::WrapUnsafe(&GetMediumOrLargeCord()); byte_string.RemoveSuffix(byte_string.size() - kSmallByteStringCapacity); - EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); + EXPECT_EQ(GetKind(byte_string), ByteStringKind::kLarge); EXPECT_EQ(byte_string, GetMediumStringView().substr(0, kSmallByteStringCapacity)); } -TEST_P(ByteStringTest, ToStringSmall) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); +TEST_F(ByteStringTest, ToStringSmall) { + ByteString byte_string = ByteString::WrapUnsafe(GetSmallStringView()); EXPECT_EQ(byte_string.ToString(), byte_string); } -TEST_P(ByteStringTest, ToStringMedium) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); +TEST_F(ByteStringTest, ToStringMedium) { + ByteString byte_string = ByteString::WrapUnsafe(GetMediumStringView()); EXPECT_EQ(byte_string.ToString(), byte_string); } -TEST_P(ByteStringTest, ToStringLarge) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); +TEST_F(ByteStringTest, ToStringLarge) { + ByteString byte_string = ByteString::WrapUnsafe(&GetMediumOrLargeCord()); EXPECT_EQ(byte_string.ToString(), byte_string); } -TEST_P(ByteStringTest, ToStringViewSmall) { +TEST_F(ByteStringTest, ToStringViewSmall) { std::string scratch; - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); + ByteString byte_string = ByteString::WrapUnsafe(GetSmallStringView()); EXPECT_EQ(byte_string.ToStringView(&scratch), GetSmallStringView()); } -TEST_P(ByteStringTest, ToStringViewMedium) { +TEST_F(ByteStringTest, ToStringViewMedium) { std::string scratch; - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); + ByteString byte_string = ByteString::WrapUnsafe(GetMediumStringView()); EXPECT_EQ(byte_string.ToStringView(&scratch), GetMediumStringView()); } -TEST_P(ByteStringTest, ToStringViewLarge) { +TEST_F(ByteStringTest, ToStringViewLarge) { std::string scratch; - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); + ByteString byte_string = ByteString::WrapUnsafe(&GetMediumOrLargeCord()); EXPECT_EQ(byte_string.ToStringView(&scratch), GetMediumOrLargeCord()); } -TEST_P(ByteStringTest, AsStringViewSmall) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); +TEST_F(ByteStringTest, AsStringViewSmall) { + ByteString byte_string = ByteString::WrapUnsafe(GetSmallStringView()); EXPECT_EQ(byte_string.AsStringView(), GetSmallStringView()); } -TEST_P(ByteStringTest, AsStringViewMedium) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); +TEST_F(ByteStringTest, AsStringViewMedium) { + ByteString byte_string = ByteString::WrapUnsafe(GetMediumStringView()); EXPECT_EQ(byte_string.AsStringView(), GetMediumStringView()); } -TEST_P(ByteStringTest, AsStringViewLarge) { - ByteString byte_string = ByteString(GetMediumOrLargeCord()); +TEST_F(ByteStringTest, AsStringViewLarge) { + ByteString byte_string = ByteString::WrapUnsafe(&GetMediumOrLargeCord()); EXPECT_DEATH(byte_string.AsStringView(), _); } -TEST_P(ByteStringTest, CopyToStringSmall) { +TEST_F(ByteStringTest, CopyToStringSmall) { std::string out; - ByteString(GetAllocator(), GetSmallStringView()).CopyToString(&out); + ByteString::WrapUnsafe(GetSmallStringView()).CopyToString(&out); EXPECT_EQ(out, GetSmallStringView()); } -TEST_P(ByteStringTest, CopyToStringMedium) { +TEST_F(ByteStringTest, CopyToStringMedium) { std::string out; - ByteString(GetAllocator(), GetMediumStringView()).CopyToString(&out); + ByteString::WrapUnsafe(GetMediumStringView()).CopyToString(&out); EXPECT_EQ(out, GetMediumStringView()); } -TEST_P(ByteStringTest, CopyToStringLarge) { +TEST_F(ByteStringTest, CopyToStringLarge) { std::string out; - ByteString(GetAllocator(), GetMediumOrLargeCord()).CopyToString(&out); + ByteString::WrapUnsafe(&GetMediumOrLargeCord()).CopyToString(&out); EXPECT_EQ(out, GetMediumOrLargeCord()); } -TEST_P(ByteStringTest, AppendToStringSmall) { +TEST_F(ByteStringTest, AppendToStringSmall) { std::string out; - ByteString(GetAllocator(), GetSmallStringView()).AppendToString(&out); + ByteString::WrapUnsafe(GetSmallStringView()).AppendToString(&out); EXPECT_EQ(out, GetSmallStringView()); } -TEST_P(ByteStringTest, AppendToStringMedium) { +TEST_F(ByteStringTest, AppendToStringMedium) { std::string out; - ByteString(GetAllocator(), GetMediumStringView()).AppendToString(&out); + ByteString::WrapUnsafe(GetMediumStringView()).AppendToString(&out); EXPECT_EQ(out, GetMediumStringView()); } -TEST_P(ByteStringTest, AppendToStringLarge) { +TEST_F(ByteStringTest, AppendToStringLarge) { std::string out; - ByteString(GetAllocator(), GetMediumOrLargeCord()).AppendToString(&out); + ByteString::WrapUnsafe(&GetMediumOrLargeCord()).AppendToString(&out); EXPECT_EQ(out, GetMediumOrLargeCord()); } -TEST_P(ByteStringTest, ToCordSmall) { - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); +TEST_F(ByteStringTest, ToCordSmall) { + ByteString byte_string = ByteString::WrapUnsafe(GetSmallStringView()); EXPECT_EQ(byte_string.ToCord(), byte_string); EXPECT_EQ(std::move(byte_string).ToCord(), GetSmallStringView()); } -TEST_P(ByteStringTest, ToCordMedium) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); +TEST_F(ByteStringTest, ToCordMedium) { + ByteString byte_string = ByteString::WrapUnsafe(GetMediumStringView()); EXPECT_EQ(byte_string.ToCord(), byte_string); EXPECT_EQ(std::move(byte_string).ToCord(), GetMediumStringView()); } -TEST_P(ByteStringTest, ToCordLarge) { - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); +TEST_F(ByteStringTest, ToCordLarge) { + ByteString byte_string = ByteString::WrapUnsafe(&GetMediumOrLargeCord()); EXPECT_EQ(byte_string.ToCord(), byte_string); EXPECT_EQ(std::move(byte_string).ToCord(), GetMediumOrLargeCord()); } -TEST_P(ByteStringTest, CopyToCordSmall) { +TEST_F(ByteStringTest, CopyToCordSmall) { absl::Cord out; - ByteString(GetAllocator(), GetSmallStringView()).CopyToCord(&out); + ByteString::WrapUnsafe(GetSmallStringView()).CopyToCord(&out); EXPECT_EQ(out, GetSmallStringView()); } -TEST_P(ByteStringTest, CopyToCordMedium) { +TEST_F(ByteStringTest, CopyToCordMedium) { absl::Cord out; - ByteString(GetAllocator(), GetMediumStringView()).CopyToCord(&out); + ByteString::WrapUnsafe(GetMediumStringView()).CopyToCord(&out); EXPECT_EQ(out, GetMediumStringView()); } -TEST_P(ByteStringTest, CopyToCordLarge) { +TEST_F(ByteStringTest, CopyToCordLarge) { absl::Cord out; - ByteString(GetAllocator(), GetMediumOrLargeCord()).CopyToCord(&out); + ByteString::WrapUnsafe(&GetMediumOrLargeCord()).CopyToCord(&out); EXPECT_EQ(out, GetMediumOrLargeCord()); } -TEST_P(ByteStringTest, AppendToCordSmall) { +TEST_F(ByteStringTest, AppendToCordSmall) { absl::Cord out; - ByteString(GetAllocator(), GetSmallStringView()).AppendToCord(&out); + ByteString::WrapUnsafe(GetSmallStringView()).AppendToCord(&out); EXPECT_EQ(out, GetSmallStringView()); } -TEST_P(ByteStringTest, AppendToCordMedium) { +TEST_F(ByteStringTest, AppendToCordMedium) { absl::Cord out; - ByteString(GetAllocator(), GetMediumStringView()).AppendToCord(&out); + ByteString::WrapUnsafe(GetMediumStringView()).AppendToCord(&out); EXPECT_EQ(out, GetMediumStringView()); } -TEST_P(ByteStringTest, AppendToCordLarge) { +TEST_F(ByteStringTest, AppendToCordLarge) { absl::Cord out; - ByteString(GetAllocator(), GetMediumOrLargeCord()).AppendToCord(&out); + ByteString::WrapUnsafe(&GetMediumOrLargeCord()).AppendToCord(&out); EXPECT_EQ(out, GetMediumOrLargeCord()); } -TEST_P(ByteStringTest, CloneSmall) { +TEST_F(ByteStringTest, CloneSmall) { google::protobuf::Arena arena; - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); + ByteString byte_string = ByteString::WrapUnsafe(GetSmallStringView()); EXPECT_EQ(byte_string.Clone(&arena), byte_string); } -TEST_P(ByteStringTest, CloneMedium) { +TEST_F(ByteStringTest, CloneMedium) { google::protobuf::Arena arena; - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); + ByteString byte_string = ByteString::WrapUnsafe(GetMediumStringView()); EXPECT_EQ(byte_string.Clone(&arena), byte_string); } -TEST_P(ByteStringTest, CloneLarge) { +TEST_F(ByteStringTest, CloneLarge) { google::protobuf::Arena arena; - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); + ByteString byte_string = ByteString::WrapUnsafe(&GetMediumOrLargeCord()); EXPECT_EQ(byte_string.Clone(&arena), byte_string); } -TEST_P(ByteStringTest, LegacyByteStringSmall) { +TEST_F(ByteStringTest, LegacyByteStringSmall) { google::protobuf::Arena arena; - ByteString byte_string = ByteString(GetAllocator(), GetSmallStringView()); + ByteString byte_string = ByteString::WrapUnsafe(GetSmallStringView()); EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/false, &arena), GetSmallStringView()); EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/true, &arena), GetSmallStringView()); } -TEST_P(ByteStringTest, LegacyByteStringMedium) { +TEST_F(ByteStringTest, LegacyByteStringMedium) { google::protobuf::Arena arena; - ByteString byte_string = ByteString(GetAllocator(), GetMediumStringView()); + ByteString byte_string = ByteString::WrapUnsafe(GetMediumStringView()); EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/false, &arena), GetMediumStringView()); EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/true, &arena), GetMediumStringView()); } -TEST_P(ByteStringTest, LegacyByteStringLarge) { +TEST_F(ByteStringTest, LegacyByteStringLarge) { google::protobuf::Arena arena; - ByteString byte_string = ByteString(GetAllocator(), GetMediumOrLargeCord()); + ByteString byte_string = ByteString::WrapUnsafe(&GetMediumOrLargeCord()); EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/false, &arena), GetMediumOrLargeCord()); EXPECT_EQ(LegacyByteString(byte_string, /*stable=*/true, &arena), GetMediumOrLargeCord()); } -TEST_P(ByteStringTest, HashValue) { - EXPECT_EQ(absl::HashOf(ByteString(GetAllocator(), GetSmallStringView())), +TEST_F(ByteStringTest, HashValue) { + EXPECT_EQ(absl::HashOf(ByteString::WrapUnsafe(GetSmallStringView())), absl::HashOf(GetSmallStringView())); - EXPECT_EQ(absl::HashOf(ByteString(GetAllocator(), GetMediumStringView())), + EXPECT_EQ(absl::HashOf(ByteString::WrapUnsafe(GetMediumStringView())), absl::HashOf(GetMediumStringView())); - EXPECT_EQ(absl::HashOf(ByteString(GetAllocator(), GetMediumOrLargeCord())), + EXPECT_EQ(absl::HashOf(ByteString::WrapUnsafe(&GetMediumOrLargeCord())), absl::HashOf(GetMediumOrLargeCord())); } -INSTANTIATE_TEST_SUITE_P(ByteStringTest, ByteStringTest, - ::testing::Values(AllocatorKind::kNewDelete, - AllocatorKind::kArena)); - } // namespace } // namespace cel::common_internal diff --git a/common/internal/value_conversion.cc b/common/internal/value_conversion.cc index 57cf2224b..ef4dda231 100644 --- a/common/internal/value_conversion.cc +++ b/common/internal/value_conversion.cc @@ -224,9 +224,9 @@ absl::StatusOr FromExprValue( case ExprValueKind::kDoubleValue: return cel::DoubleValue(value.double_value()); case ExprValueKind::kStringValue: - return cel::StringValue(value.string_value()); + return cel::StringValue::From(value.string_value(), arena); case ExprValueKind::kBytesValue: - return cel::BytesValue(value.bytes_value()); + return cel::BytesValue::From(value.bytes_value(), arena); case ExprValueKind::kNullValue: return cel::NullValue(); case ExprValueKind::kObjectValue: diff --git a/common/legacy_value.cc b/common/legacy_value.cc index 3a2108c4d..7244c437d 100644 --- a/common/legacy_value.cc +++ b/common/legacy_value.cc @@ -1043,12 +1043,10 @@ absl::Status ModernValue(google::protobuf::Arena* arena, result = DoubleValue{legacy_value.DoubleOrDie()}; return absl::OkStatus(); case CelValue::Type::kString: - result = StringValue(Borrower::Arena(arena), - legacy_value.StringOrDie().value()); + result = StringValue::Wrap(legacy_value.StringOrDie().value(), nullptr); return absl::OkStatus(); case CelValue::Type::kBytes: - result = - BytesValue(Borrower::Arena(arena), legacy_value.BytesOrDie().value()); + result = BytesValue::Wrap(legacy_value.BytesOrDie().value(), nullptr); return absl::OkStatus(); case CelValue::Type::kMessage: { auto message_wrapper = legacy_value.MessageWrapperOrDie(); @@ -1163,11 +1161,9 @@ absl::StatusOr FromLegacyValue(google::protobuf::Arena* arena, case CelValue::Type::kDouble: return DoubleValue(legacy_value.DoubleOrDie()); case CelValue::Type::kString: - return StringValue(Borrower::Arena(arena), - legacy_value.StringOrDie().value()); + return StringValue::Wrap(legacy_value.StringOrDie().value(), nullptr); case CelValue::Type::kBytes: - return BytesValue(Borrower::Arena(arena), - legacy_value.BytesOrDie().value()); + return BytesValue::Wrap(legacy_value.BytesOrDie().value(), nullptr); case CelValue::Type::kMessage: { auto message_wrapper = legacy_value.MessageWrapperOrDie(); return common_internal::MakeLegacyStructValue( diff --git a/common/type_reflector_test.cc b/common/type_reflector_test.cc index d9c855e4b..961de0d8e 100644 --- a/common/type_reflector_test.cc +++ b/common/type_reflector_test.cc @@ -32,6 +32,7 @@ namespace cel { namespace { +using ::absl_testing::IsOk; using ::absl_testing::IsOkAndHolds; using ::absl_testing::StatusIs; using ::cel::test::ErrorValueIs; @@ -178,8 +179,8 @@ TEST_F(TypeReflectorTest, NewMapValueBuilderCoverage_DynamicDynamic) { EXPECT_OK(builder->Put(IntValue(1), IntValue(4))); EXPECT_OK(builder->Put(UintValue(0), IntValue(5))); EXPECT_OK(builder->Put(UintValue(1), IntValue(6))); - EXPECT_OK(builder->Put(StringValue("a"), IntValue(7))); - EXPECT_OK(builder->Put(StringValue("b"), IntValue(8))); + EXPECT_THAT(builder->Put(StringValue::WrapUnsafe("a"), IntValue(7)), IsOk()); + EXPECT_THAT(builder->Put(StringValue::WrapUnsafe("b"), IntValue(8)), IsOk()); EXPECT_EQ(builder->Size(), 8); EXPECT_FALSE(builder->IsEmpty()); auto value = std::move(*builder).Build(); @@ -407,17 +408,18 @@ TEST_F(TypeReflectorTest, NewValueBuilder_StringValue) { arena(), internal::GetTestingDescriptorPool(), internal::GetTestingMessageFactory(), "google.protobuf.StringValue"); ASSERT_THAT(builder, NotNull()); - EXPECT_THAT(builder->SetFieldByName("value", StringValue("foo")), + EXPECT_THAT(builder->SetFieldByName("value", StringValue::WrapUnsafe("foo")), IsOkAndHolds(Eq(std::nullopt))); - EXPECT_THAT(builder->SetFieldByName("does_not_exist", StringValue("foo")), - IsOkAndHolds(Optional( - ErrorValueIs(StatusIs(absl::StatusCode::kNotFound))))); + EXPECT_THAT( + builder->SetFieldByName("does_not_exist", StringValue::WrapUnsafe("foo")), + IsOkAndHolds( + Optional(ErrorValueIs(StatusIs(absl::StatusCode::kNotFound))))); EXPECT_THAT(builder->SetFieldByName("value", BoolValue(true)), IsOkAndHolds(Optional( ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument))))); - EXPECT_THAT(builder->SetFieldByNumber(1, StringValue("foo")), + EXPECT_THAT(builder->SetFieldByNumber(1, StringValue::WrapUnsafe("foo")), IsOkAndHolds(Eq(std::nullopt))); - EXPECT_THAT(builder->SetFieldByNumber(2, StringValue("foo")), + EXPECT_THAT(builder->SetFieldByNumber(2, StringValue::WrapUnsafe("foo")), IsOkAndHolds(Optional( ErrorValueIs(StatusIs(absl::StatusCode::kNotFound))))); EXPECT_THAT(builder->SetFieldByNumber(1, BoolValue(true)), @@ -433,17 +435,18 @@ TEST_F(TypeReflectorTest, NewValueBuilder_BytesValue) { arena(), internal::GetTestingDescriptorPool(), internal::GetTestingMessageFactory(), "google.protobuf.BytesValue"); ASSERT_THAT(builder, NotNull()); - EXPECT_THAT(builder->SetFieldByName("value", BytesValue("foo")), + EXPECT_THAT(builder->SetFieldByName("value", BytesValue::WrapUnsafe("foo")), IsOkAndHolds(Eq(std::nullopt))); - EXPECT_THAT(builder->SetFieldByName("does_not_exist", BytesValue("foo")), - IsOkAndHolds(Optional( - ErrorValueIs(StatusIs(absl::StatusCode::kNotFound))))); + EXPECT_THAT( + builder->SetFieldByName("does_not_exist", BytesValue::WrapUnsafe("foo")), + IsOkAndHolds( + Optional(ErrorValueIs(StatusIs(absl::StatusCode::kNotFound))))); EXPECT_THAT(builder->SetFieldByName("value", BoolValue(true)), IsOkAndHolds(Optional( ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument))))); - EXPECT_THAT(builder->SetFieldByNumber(1, BytesValue("foo")), + EXPECT_THAT(builder->SetFieldByNumber(1, BytesValue::WrapUnsafe("foo")), IsOkAndHolds(Eq(std::nullopt))); - EXPECT_THAT(builder->SetFieldByNumber(2, BytesValue("foo")), + EXPECT_THAT(builder->SetFieldByNumber(2, BytesValue::WrapUnsafe("foo")), IsOkAndHolds(Optional( ErrorValueIs(StatusIs(absl::StatusCode::kNotFound))))); EXPECT_THAT(builder->SetFieldByNumber(1, BoolValue(true)), @@ -549,10 +552,11 @@ TEST_F(TypeReflectorTest, NewValueBuilder_Any) { arena(), internal::GetTestingDescriptorPool(), internal::GetTestingMessageFactory(), "google.protobuf.Any"); ASSERT_THAT(builder, NotNull()); - EXPECT_THAT(builder->SetFieldByName( - "type_url", - StringValue("type.googleapis.com/google.protobuf.BoolValue")), - IsOkAndHolds(Eq(std::nullopt))); + EXPECT_THAT( + builder->SetFieldByName( + "type_url", StringValue::WrapUnsafe( + "type.googleapis.com/google.protobuf.BoolValue")), + IsOkAndHolds(Eq(std::nullopt))); EXPECT_THAT(builder->SetFieldByName("does_not_exist", IntValue(1)), IsOkAndHolds(Optional( ErrorValueIs(StatusIs(absl::StatusCode::kNotFound))))); @@ -564,10 +568,10 @@ TEST_F(TypeReflectorTest, NewValueBuilder_Any) { EXPECT_THAT(builder->SetFieldByName("value", BoolValue(true)), IsOkAndHolds(Optional( ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument))))); - EXPECT_THAT( - builder->SetFieldByNumber( - 1, StringValue("type.googleapis.com/google.protobuf.BoolValue")), - IsOkAndHolds(Eq(std::nullopt))); + EXPECT_THAT(builder->SetFieldByNumber( + 1, StringValue::WrapUnsafe( + "type.googleapis.com/google.protobuf.BoolValue")), + IsOkAndHolds(Eq(std::nullopt))); EXPECT_THAT(builder->SetFieldByNumber(3, IntValue(1)), IsOkAndHolds(Optional( ErrorValueIs(StatusIs(absl::StatusCode::kNotFound))))); diff --git a/common/value.cc b/common/value.cc index a2ffea620..fd948c500 100644 --- a/common/value.cc +++ b/common/value.cc @@ -52,6 +52,7 @@ #include "internal/number.h" #include "internal/protobuf_runtime_version.h" #include "internal/status_macros.h" +#include "internal/utf8.h" #include "internal/well_known_types.h" #include "runtime/runtime_options.h" #include "google/protobuf/arena.h" @@ -422,10 +423,14 @@ void StringMapFieldKeyAccessor(const google::protobuf::MapKey& key, ABSL_DCHECK(result != nullptr); #if CEL_INTERNAL_PROTOBUF_OSS_VERSION_PREREQ(5, 30, 0) - *result = StringValue(Borrower::Arena(MessageArenaOr(message, arena)), - key.GetStringValue()); + if (google::protobuf::Arena* message_arena = message->GetArena(); + message_arena != nullptr) { + *result = StringValue::Wrap(key.GetStringValue(), message_arena); + } else { + *result = StringValue::From(key.GetStringValue(), arena); + } #else - *result = StringValue(arena, key.GetStringValue()); + *result = StringValue::From(key.GetStringValue(), arena); #endif } @@ -604,9 +609,9 @@ void StringMapFieldValueAccessor( ABSL_DCHECK_EQ(field->type(), google::protobuf::FieldDescriptor::TYPE_STRING); if (message->GetArena() == nullptr) { - *result = StringValue(arena, value.GetStringValue()); + *result = StringValue::From(value.GetStringValue(), arena); } else { - *result = StringValue(Borrower::Arena(arena), value.GetStringValue()); + *result = StringValue::Wrap(value.GetStringValue(), message->GetArena()); } } @@ -647,9 +652,9 @@ void BytesMapFieldValueAccessor( ABSL_DCHECK_EQ(field->type(), google::protobuf::FieldDescriptor::TYPE_BYTES); if (message->GetArena() == nullptr) { - *result = BytesValue(arena, value.GetStringValue()); + *result = BytesValue::From(value.GetStringValue(), arena); } else { - *result = BytesValue(Borrower::Arena(arena), value.GetStringValue()); + *result = BytesValue::Wrap(value.GetStringValue(), message->GetArena()); } } @@ -940,16 +945,18 @@ void StringRepeatedFieldAccessor( [&](absl::string_view string) { if (string.data() == scratch.data() && string.size() == scratch.size()) { - *result = StringValue(arena, std::move(scratch)); + *result = StringValue::From(std::move(scratch), arena); } else { if (message->GetArena() == nullptr) { - *result = StringValue(arena, string); + *result = StringValue::From(string, arena); } else { - *result = StringValue(Borrower::Arena(arena), string); + *result = StringValue::Wrap(string, message->GetArena()); } } }, - [&](absl::Cord&& cord) { *result = StringValue(std::move(cord)); }), + [&](absl::Cord&& cord) { + *result = StringValue::From(std::move(cord), arena); + }), well_known_types::AsVariant(well_known_types::GetRepeatedStringField( *message, field, index, scratch))); } @@ -1007,16 +1014,18 @@ void BytesRepeatedFieldAccessor( [&](absl::string_view string) { if (string.data() == scratch.data() && string.size() == scratch.size()) { - *result = BytesValue(arena, std::move(scratch)); + *result = BytesValue::From(std::move(scratch), arena); } else { if (message->GetArena() == nullptr) { - *result = BytesValue(arena, string); + *result = BytesValue::From(string, arena); } else { - *result = BytesValue(Borrower::Arena(arena), string); + *result = BytesValue::Wrap(string, message->GetArena()); } } }, - [&](absl::Cord&& cord) { *result = BytesValue(std::move(cord)); }), + [&](absl::Cord&& cord) { + *result = BytesValue::From(std::move(cord), arena); + }), well_known_types::AsVariant(well_known_types::GetRepeatedBytesField( *message, field, index, scratch))); } @@ -1166,15 +1175,16 @@ struct OwningWellKnownTypesValueVisitor { } if (scratch->data() == string.data() && scratch->size() == string.size()) { - return BytesValue(arena, std::move(*scratch)); + return BytesValue::From(std::move(*scratch), + arena); } - return BytesValue(arena, string); + return BytesValue::From(string, arena); }, [&](absl::Cord&& cord) -> BytesValue { if (cord.empty()) { return BytesValue(); } - return BytesValue(arena, cord); + return BytesValue::From(cord, arena); }), well_known_types::AsVariant(std::move(value))); } @@ -1187,15 +1197,16 @@ struct OwningWellKnownTypesValueVisitor { } if (scratch->data() == string.data() && scratch->size() == string.size()) { - return StringValue(arena, std::move(*scratch)); + return StringValue::From(std::move(*scratch), + arena); } - return StringValue(arena, string); + return StringValue::From(string, arena); }, [&](absl::Cord&& cord) -> StringValue { if (cord.empty()) { return StringValue(); } - return StringValue(arena, cord); + return StringValue::From(cord, arena); }), well_known_types::AsVariant(std::move(value))); } @@ -1264,14 +1275,17 @@ struct BorrowingWellKnownTypesValueVisitor { [&](absl::string_view string) -> BytesValue { if (string.data() == scratch->data() && string.size() == scratch->size()) { - return BytesValue(arena, std::move(*scratch)); + return BytesValue::From(std::move(*scratch), arena); } else { - return BytesValue( - Borrower::Arena(MessageArenaOr(message, arena)), string); + if (google::protobuf::Arena* message_arena = message->GetArena(); + message_arena != nullptr) { + return BytesValue::Wrap(string, message_arena); + } + return BytesValue::From(string, arena); } }, [&](absl::Cord&& cord) -> BytesValue { - return BytesValue(std::move(cord)); + return BytesValue::From(std::move(cord), arena); }), well_known_types::AsVariant(std::move(value))); } @@ -1282,14 +1296,17 @@ struct BorrowingWellKnownTypesValueVisitor { [&](absl::string_view string) -> StringValue { if (string.data() == scratch->data() && string.size() == scratch->size()) { - return StringValue(arena, std::move(*scratch)); + return StringValue::From(std::move(*scratch), arena); } else { - return StringValue( - Borrower::Arena(MessageArenaOr(message, arena)), string); + if (google::protobuf::Arena* message_arena = message->GetArena(); + message_arena != nullptr) { + return StringValue::Wrap(string, message_arena); + } + return StringValue::From(string, arena); } }, [&](absl::Cord&& cord) -> StringValue { - return StringValue(std::move(cord)); + return StringValue::From(std::move(cord), arena); }), well_known_types::AsVariant(std::move(value))); } @@ -1563,17 +1580,20 @@ Value WrapFieldImpl( [&](absl::string_view string) -> StringValue { if (string.data() == scratch.data() && string.size() == scratch.size()) { - return StringValue(arena, std::move(scratch)); + return StringValue::From(std::move(scratch), arena); } if constexpr (Unsafe::value) { return StringValue::WrapUnsafe(string); } else { - return StringValue( - Borrower::Arena(MessageArenaOr(message, arena)), string); + if (google::protobuf::Arena* message_arena = message->GetArena(); + message_arena != nullptr) { + return StringValue::Wrap(string, message_arena); + } + return StringValue::From(string, arena); } }, [&](absl::Cord&& cord) -> StringValue { - return StringValue(std::move(cord)); + return StringValue::From(std::move(cord), arena); }), well_known_types::AsVariant( well_known_types::GetStringField(*message, field, scratch))); @@ -1602,17 +1622,20 @@ Value WrapFieldImpl( [&](absl::string_view string) -> BytesValue { if (string.data() == scratch.data() && string.size() == scratch.size()) { - return BytesValue(arena, std::move(scratch)); + return BytesValue::From(std::move(scratch), arena); } if constexpr (Unsafe::value) { return BytesValue::WrapUnsafe(string); } else { - return BytesValue( - Borrower::Arena(MessageArenaOr(message, arena)), string); + if (google::protobuf::Arena* message_arena = message->GetArena(); + message_arena != nullptr) { + return BytesValue::Wrap(string, message_arena); + } + return BytesValue::From(string, arena); } }, [&](absl::Cord&& cord) -> BytesValue { - return BytesValue(std::move(cord)); + return BytesValue::From(std::move(cord), arena); }), well_known_types::AsVariant( well_known_types::GetBytesField(*message, field, scratch))); @@ -1700,17 +1723,20 @@ Value WrapRepeatedFieldImpl( [&](absl::string_view string) -> StringValue { if (string.data() == scratch.data() && string.size() == scratch.size()) { - return StringValue(arena, std::move(scratch)); + return StringValue::From(std::move(scratch), arena); } if constexpr (Unsafe::value) { return StringValue::WrapUnsafe(string); } else { - return StringValue( - Borrower::Arena(MessageArenaOr(message, arena)), string); + if (google::protobuf::Arena* message_arena = message->GetArena(); + message_arena != nullptr) { + return StringValue::Wrap(string, message_arena); + } + return StringValue::From(string, arena); } }, [&](absl::Cord&& cord) -> StringValue { - return StringValue(std::move(cord)); + return StringValue::From(std::move(cord), arena); }), well_known_types::AsVariant(well_known_types::GetRepeatedStringField( reflection, *message, field, index, scratch))); @@ -1734,17 +1760,20 @@ Value WrapRepeatedFieldImpl( [&](absl::string_view string) -> BytesValue { if (string.data() == scratch.data() && string.size() == scratch.size()) { - return BytesValue(arena, std::move(scratch)); + return BytesValue::From(std::move(scratch), arena); } if constexpr (Unsafe::value) { return BytesValue::WrapUnsafe(string); } else { - return BytesValue( - Borrower::Arena(MessageArenaOr(message, arena)), string); + if (google::protobuf::Arena* message_arena = message->GetArena(); + message_arena != nullptr) { + return BytesValue::Wrap(string, message_arena); + } + return BytesValue::From(string, arena); } }, [&](absl::Cord&& cord) -> BytesValue { - return BytesValue(std::move(cord)); + return BytesValue::From(std::move(cord), arena); }), well_known_types::AsVariant(well_known_types::GetRepeatedBytesField( reflection, *message, field, index, scratch))); @@ -1810,8 +1839,7 @@ Value WrapMapFieldValueImpl( if constexpr (Unsafe::value) { return StringValue::WrapUnsafe(value.GetStringValue()); } else { - return StringValue(Borrower::Arena(MessageArenaOr(message, arena)), - value.GetStringValue()); + return StringValue::From(value.GetStringValue(), arena); } case google::protobuf::FieldDescriptor::TYPE_GROUP: ABSL_FALLTHROUGH_INTENDED; @@ -1827,8 +1855,7 @@ Value WrapMapFieldValueImpl( if constexpr (Unsafe::value) { return BytesValue::WrapUnsafe(value.GetStringValue()); } else { - return BytesValue(Borrower::Arena(MessageArenaOr(message, arena)), - value.GetStringValue()); + return BytesValue::From(value.GetStringValue(), arena); } case google::protobuf::FieldDescriptor::TYPE_FIXED32: ABSL_FALLTHROUGH_INTENDED; @@ -1913,11 +1940,12 @@ StringValue Value::WrapMapFieldKeyString( ABSL_DCHECK_EQ(key.type(), google::protobuf::FieldDescriptor::CPPTYPE_STRING); #if CEL_INTERNAL_PROTOBUF_OSS_VERSION_PREREQ(5, 30, 0) - return StringValue(Borrower::Arena(MessageArenaOr(message, arena)), - key.GetStringValue()); -#else - return StringValue(arena, key.GetStringValue()); + if (google::protobuf::Arena* message_arena = message->GetArena(); + message_arena != nullptr) { + return StringValue::Wrap(key.GetStringValue(), message_arena); + } #endif + return StringValue::From(key.GetStringValue(), arena); } Value Value::WrapMapFieldValue( @@ -2801,4 +2829,14 @@ absl::StatusOr ValueIterator::Next1( return false; } +StringValue::StringValue(const BytesValue& other) : StringValue(other.value_) { + ABSL_DCHECK(value_.Visit(absl::Overload( + [](absl::string_view string) -> bool { + return internal::Utf8IsValid(string); + }, + [](const absl::Cord& cord) -> bool { + return internal::Utf8IsValid(cord); + }))); +} + } // namespace cel diff --git a/common/value.h b/common/value.h index f6ce5de1a..d1a793b25 100644 --- a/common/value.h +++ b/common/value.h @@ -2941,6 +2941,9 @@ absl::StatusOr RepeatedFieldAccessorFor( } // namespace common_internal +inline BytesValue::BytesValue(const StringValue& other) + : BytesValue(other.value_) {} + } // namespace cel #pragma pop_macro("GetMessage") diff --git a/common/value_testing_test.cc b/common/value_testing_test.cc index d7a7a4c07..425ce92cc 100644 --- a/common/value_testing_test.cc +++ b/common/value_testing_test.cc @@ -25,6 +25,7 @@ namespace cel::test { namespace { +using ::absl_testing::IsOk; using ::absl_testing::StatusIs; using ::testing::_; using ::testing::ElementsAre; @@ -124,11 +125,12 @@ TEST(TimestampValueIs, NonMatchMessage) { } TEST(StringValueIs, Match) { - EXPECT_THAT(StringValue("hello!"), StringValueIs("hello!")); + EXPECT_THAT(StringValue::WrapUnsafe("hello!"), StringValueIs("hello!")); } TEST(StringValueIs, NoMatch) { - EXPECT_THAT(StringValue("hello!"), Not(StringValueIs("goodbye!"))); + EXPECT_THAT(StringValue::WrapUnsafe("hello!"), + Not(StringValueIs("goodbye!"))); EXPECT_THAT(IntValue(2), Not(StringValueIs("goodbye!"))); } @@ -139,11 +141,11 @@ TEST(StringValueIs, NonMatchMessage) { } TEST(BytesValueIs, Match) { - EXPECT_THAT(BytesValue("hello!"), BytesValueIs("hello!")); + EXPECT_THAT(BytesValue::WrapUnsafe("hello!"), BytesValueIs("hello!")); } TEST(BytesValueIs, NoMatch) { - EXPECT_THAT(BytesValue("hello!"), Not(BytesValueIs("goodbye!"))); + EXPECT_THAT(BytesValue::WrapUnsafe("hello!"), Not(BytesValueIs("goodbye!"))); EXPECT_THAT(IntValue(2), Not(BytesValueIs("goodbye!"))); } @@ -265,8 +267,10 @@ TEST_F(ValueMatcherTest, MapMatcherBasic) { TEST_F(ValueMatcherTest, MapMatcherMatchesElements) { auto builder = NewMapValueBuilder(arena()); - ASSERT_OK(builder->Put(IntValue(42), StringValue("answer"))); - ASSERT_OK(builder->Put(IntValue(1337), StringValue("leet"))); + ASSERT_THAT(builder->Put(IntValue(42), StringValue::WrapUnsafe("answer")), + IsOk()); + ASSERT_THAT(builder->Put(IntValue(1337), StringValue::WrapUnsafe("leet")), + IsOk()); EXPECT_THAT( std::move(*builder).Build(), MapValueIs(MapValueElements( diff --git a/common/values/bytes_value.h b/common/values/bytes_value.h index c18381a6a..461e5d8c6 100644 --- a/common/values/bytes_value.h +++ b/common/values/bytes_value.h @@ -26,15 +26,11 @@ #include "absl/base/attributes.h" #include "absl/base/nullability.h" -#include "absl/log/absl_check.h" #include "absl/status/status.h" #include "absl/strings/cord.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" -#include "common/allocator.h" -#include "common/arena.h" #include "common/internal/byte_string.h" -#include "common/memory.h" #include "common/type.h" #include "common/value_kind.h" #include "common/values/values.h" @@ -62,75 +58,61 @@ class BytesValue final : private common_internal::ValueMixin { static BytesValue From(const char* absl_nullable value, google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND); + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return BytesValue(common_internal::ByteString::From(value, arena)); + } static BytesValue From(absl::string_view value, google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND); - static BytesValue From(const absl::Cord& value); + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return BytesValue(common_internal::ByteString::From(value, arena)); + } + static BytesValue From(const absl::Cord& value, + google::protobuf::Arena* absl_nonnull arena + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return BytesValue(common_internal::ByteString::From(value, arena)); + } static BytesValue From(std::string&& value, google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND); + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return BytesValue( + common_internal::ByteString::From(std::move(value), arena)); + } static BytesValue Wrap(absl::string_view value, google::protobuf::Arena* absl_nullable arena - ABSL_ATTRIBUTE_LIFETIME_BOUND); - static BytesValue Wrap(absl::string_view value) = delete; - static BytesValue Wrap(const absl::Cord& value); - static BytesValue Wrap(std::string&& value) = delete; - static BytesValue Wrap(std::string&& value, - google::protobuf::Arena* absl_nullable arena - ABSL_ATTRIBUTE_LIFETIME_BOUND) = delete; + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return BytesValue(common_internal::ByteString::Wrap(value, arena)); + } + static BytesValue Wrap( + const absl::Cord* absl_nonnull value ABSL_ATTRIBUTE_LIFETIME_BOUND, + google::protobuf::Arena* absl_nullable arena ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return BytesValue(common_internal::ByteString::Wrap(value, arena)); + } + static BytesValue Wrap(std::nullptr_t, google::protobuf::Arena*) = delete; + static BytesValue Wrap(std::string&& value, google::protobuf::Arena*) = delete; // Returns a BytesValue that aliases the provided string. Caller must ensure // the provided string outlives the use of the returned BytesValue. - static BytesValue WrapUnsafe(absl::string_view value); + static BytesValue WrapUnsafe(absl::string_view value) { + return BytesValue(common_internal::ByteString::WrapUnsafe(value)); + } + static BytesValue WrapUnsafe(const absl::Cord* absl_nonnull value) { + return BytesValue(common_internal::ByteString::WrapUnsafe(value)); + } + static BytesValue WrapUnsafe(std::nullptr_t) = delete; static BytesValue Concat(const BytesValue& lhs, const BytesValue& rhs, google::protobuf::Arena* absl_nonnull arena ABSL_ATTRIBUTE_LIFETIME_BOUND); - ABSL_DEPRECATED("Use From") - explicit BytesValue(const char* absl_nullable value) : value_(value) {} - - ABSL_DEPRECATED("Use From") - explicit BytesValue(absl::string_view value) : value_(value) {} - - ABSL_DEPRECATED("Use From") - explicit BytesValue(const absl::Cord& value) : value_(value) {} - - ABSL_DEPRECATED("Use From") - explicit BytesValue(std::string&& value) : value_(std::move(value)) {} - - ABSL_DEPRECATED("Use From") - BytesValue(Allocator<> allocator, const char* absl_nullable value) - : value_(allocator, value) {} - - ABSL_DEPRECATED("Use From") - BytesValue(Allocator<> allocator, absl::string_view value) - : value_(allocator, value) {} - - ABSL_DEPRECATED("Use From") - BytesValue(Allocator<> allocator, const absl::Cord& value) - : value_(allocator, value) {} - - ABSL_DEPRECATED("Use From") - BytesValue(Allocator<> allocator, std::string&& value) - : value_(allocator, std::move(value)) {} - - ABSL_DEPRECATED("Use Wrap") - BytesValue(Borrower borrower, absl::string_view value) - : value_(borrower, value) {} - - ABSL_DEPRECATED("Use Wrap") - BytesValue(Borrower borrower, const absl::Cord& value) - : value_(borrower, value) {} - BytesValue() = default; BytesValue(const BytesValue&) = default; BytesValue(BytesValue&&) = default; BytesValue& operator=(const BytesValue&) = default; BytesValue& operator=(BytesValue&&) = default; + explicit BytesValue(const StringValue& other); + constexpr ValueKind kind() const { return kKind; } absl::string_view GetTypeName() const { return BytesType::kName; } @@ -185,9 +167,9 @@ class BytesValue final : private common_internal::ValueMixin { return value_.Visit(std::forward(visitor)); } - void swap(BytesValue& other) noexcept { + friend void swap(BytesValue& lhs, BytesValue& rhs) noexcept { using std::swap; - swap(value_, other.value_); + swap(lhs.value_, rhs.value_); } size_t Size() const; @@ -238,12 +220,12 @@ class BytesValue final : private common_internal::ValueMixin { } private: + friend class StringValue; friend class common_internal::ValueMixin; friend class BytesValueInputStream; friend class BytesValueOutputStream; friend absl::string_view common_internal::LegacyBytesValue( const BytesValue& value, bool stable, google::protobuf::Arena* absl_nonnull arena); - friend struct ArenaTraits; explicit BytesValue(common_internal::ByteString value) noexcept : value_(std::move(value)) {} @@ -251,8 +233,6 @@ class BytesValue final : private common_internal::ValueMixin { common_internal::ByteString value_; }; -inline void swap(BytesValue& lhs, BytesValue& rhs) noexcept { lhs.swap(rhs); } - inline std::ostream& operator<<(std::ostream& out, const BytesValue& value) { return out << value.DebugString(); } @@ -273,48 +253,6 @@ inline bool operator!=(absl::string_view lhs, const BytesValue& rhs) { return rhs != lhs; } -inline BytesValue BytesValue::From(const char* absl_nullable value, - google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND) { - return From(absl::NullSafeStringView(value), arena); -} - -inline BytesValue BytesValue::From(absl::string_view value, - google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND) { - ABSL_DCHECK(arena != nullptr); - - return BytesValue(arena, value); -} - -inline BytesValue BytesValue::From(const absl::Cord& value) { - return BytesValue(value); -} - -inline BytesValue BytesValue::From(std::string&& value, - google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND) { - ABSL_DCHECK(arena != nullptr); - - return BytesValue(arena, std::move(value)); -} - -inline BytesValue BytesValue::Wrap(absl::string_view value, - google::protobuf::Arena* absl_nullable arena - ABSL_ATTRIBUTE_LIFETIME_BOUND) { - ABSL_DCHECK(arena != nullptr); - - return BytesValue(Borrower::Arena(arena), value); -} - -inline BytesValue BytesValue::WrapUnsafe(absl::string_view value) { - return BytesValue(common_internal::ByteString::FromExternal(value)); -} - -inline BytesValue BytesValue::Wrap(const absl::Cord& value) { - return BytesValue(value); -} - namespace common_internal { inline absl::string_view LegacyBytesValue(const BytesValue& value, bool stable, @@ -324,15 +262,6 @@ inline absl::string_view LegacyBytesValue(const BytesValue& value, bool stable, } // namespace common_internal -template <> -struct ArenaTraits { - using constructible = std::true_type; - - static bool trivially_destructible(const BytesValue& value) { - return ArenaTraits<>::trivially_destructible(value.value_); - } -}; - } // namespace cel #endif // THIRD_PARTY_CEL_CPP_COMMON_VALUES_BYTES_VALUE_H_ diff --git a/common/values/bytes_value_input_stream.h b/common/values/bytes_value_input_stream.h index c4224f30d..35050d2de 100644 --- a/common/values/bytes_value_input_stream.h +++ b/common/values/bytes_value_input_stream.h @@ -49,27 +49,31 @@ class BytesValueInputStream final : public google::protobuf::io::ZeroCopyInputSt bool Next(const void** data, int* size) override { return absl::visit( [&data, &size](auto& alternative) -> bool { - return alternative.Next(data, size); + return alternative.stream.Next(data, size); }, AsVariant()); } void BackUp(int count) override { absl::visit( - [&count](auto& alternative) -> void { alternative.BackUp(count); }, + [&count](auto& alternative) -> void { + alternative.stream.BackUp(count); + }, AsVariant()); } bool Skip(int count) override { return absl::visit( - [&count](auto& alternative) -> bool { return alternative.Skip(count); }, + [&count](auto& alternative) -> bool { + return alternative.stream.Skip(count); + }, AsVariant()); } int64_t ByteCount() const override { return absl::visit( [](const auto& alternative) -> int64_t { - return alternative.ByteCount(); + return alternative.stream.ByteCount(); }, AsVariant()); } @@ -77,14 +81,25 @@ class BytesValueInputStream final : public google::protobuf::io::ZeroCopyInputSt bool ReadCord(absl::Cord* cord, int count) override { return absl::visit( [&cord, &count](auto& alternative) -> bool { - return alternative.ReadCord(cord, count); + return alternative.stream.ReadCord(cord, count); }, AsVariant()); } private: - using Variant = - absl::variant; + struct ArrayStream { + ArrayStream(const char* data, int size) : stream(data, size) {} + + google::protobuf::io::ArrayInputStream stream; + }; + struct CordStream { + explicit CordStream(const absl::Cord& cord) + : cord(cord), stream(&this->cord) {} + + absl::Cord cord; + google::protobuf::io::CordInputStream stream; + }; + using Variant = absl::variant; void Construct(const BytesValue* absl_nonnull value) { ABSL_DCHECK(value != nullptr); @@ -97,7 +112,7 @@ class BytesValueInputStream final : public google::protobuf::io::ZeroCopyInputSt Construct(value->value_.GetMedium()); break; case common_internal::ByteStringKind::kLarge: - Construct(&value->value_.GetLarge()); + Construct(value->value_.GetLarge()); break; } } @@ -106,13 +121,13 @@ class BytesValueInputStream final : public google::protobuf::io::ZeroCopyInputSt ABSL_DCHECK_LE(value.size(), static_cast(std::numeric_limits::max())); ::new (static_cast(&impl_[0])) - Variant(absl::in_place_type, value.data(), + Variant(absl::in_place_type, value.data(), static_cast(value.size())); } - void Construct(const absl::Cord* absl_nonnull value) { + void Construct(const absl::Cord& value) { ::new (static_cast(&impl_[0])) - Variant(absl::in_place_type, value); + Variant(absl::in_place_type, value); } void Destruct() { AsVariant().~variant(); } diff --git a/common/values/bytes_value_output_stream.h b/common/values/bytes_value_output_stream.h index 0773e40e7..b23d1bd3d 100644 --- a/common/values/bytes_value_output_stream.h +++ b/common/values/bytes_value_output_stream.h @@ -26,6 +26,7 @@ #include "absl/base/attributes.h" #include "absl/base/nullability.h" #include "absl/functional/overload.h" +#include "absl/log/absl_check.h" #include "absl/strings/cord.h" #include "absl/strings/string_view.h" #include "absl/types/variant.h" @@ -40,13 +41,7 @@ namespace cel { class BytesValueOutputStream final : public google::protobuf::io::ZeroCopyOutputStream { public: - explicit BytesValueOutputStream(const BytesValue& value) - : BytesValueOutputStream(value, /*arena=*/nullptr) {} - - BytesValueOutputStream(const BytesValue& value, - google::protobuf::Arena* absl_nullable arena) { - Construct(value, arena); - } + explicit BytesValueOutputStream(const BytesValue& value) { Construct(value); } bool Next(void** data, int* size) override { return absl::visit(absl::Overload( @@ -54,7 +49,7 @@ class BytesValueOutputStream final : public google::protobuf::io::ZeroCopyOutput return string.stream.Next(data, size); }, [&data, &size](Cord& cord) -> bool { - return cord.Next(data, size); + return cord.stream.Next(data, size); }), AsVariant()); } @@ -63,18 +58,19 @@ class BytesValueOutputStream final : public google::protobuf::io::ZeroCopyOutput absl::visit( absl::Overload( [&count](String& string) -> void { string.stream.BackUp(count); }, - [&count](Cord& cord) -> void { cord.BackUp(count); }), + [&count](Cord& cord) -> void { cord.stream.BackUp(count); }), AsVariant()); } int64_t ByteCount() const override { - return absl::visit( - absl::Overload( - [](const String& string) -> int64_t { - return string.stream.ByteCount(); - }, - [](const Cord& cord) -> int64_t { return cord.ByteCount(); }), - AsVariant()); + return absl::visit(absl::Overload( + [](const String& string) -> int64_t { + return string.stream.ByteCount(); + }, + [](const Cord& cord) -> int64_t { + return cord.stream.ByteCount(); + }), + AsVariant()); } bool WriteAliasedRaw(const void* data, int size) override { @@ -83,19 +79,20 @@ class BytesValueOutputStream final : public google::protobuf::io::ZeroCopyOutput return string.stream.WriteAliasedRaw(data, size); }, [&data, &size](Cord& cord) -> bool { - return cord.WriteAliasedRaw(data, size); + return cord.stream.WriteAliasedRaw(data, size); }), AsVariant()); } bool AllowsAliasing() const override { - return absl::visit( - absl::Overload( - [](const String& string) -> bool { - return string.stream.AllowsAliasing(); - }, - [](const Cord& cord) -> bool { return cord.AllowsAliasing(); }), - AsVariant()); + return absl::visit(absl::Overload( + [](const String& string) -> bool { + return string.stream.AllowsAliasing(); + }, + [](const Cord& cord) -> bool { + return cord.stream.AllowsAliasing(); + }), + AsVariant()); } bool WriteCord(const absl::Cord& out) override { @@ -104,43 +101,47 @@ class BytesValueOutputStream final : public google::protobuf::io::ZeroCopyOutput [&out](String& string) -> bool { return string.stream.WriteCord(out); }, - [&out](Cord& cord) -> bool { return cord.WriteCord(out); }), + [&out](Cord& cord) -> bool { return cord.stream.WriteCord(out); }), AsVariant()); } - BytesValue Consume() && { - return absl::visit(absl::Overload( - [](String& string) -> BytesValue { - return BytesValue(string.arena, - std::move(string.target)); - }, - [](Cord& cord) -> BytesValue { - return BytesValue(cord.Consume()); - }), - AsVariant()); + BytesValue Consume(google::protobuf::Arena* absl_nonnull arena) && { + ABSL_DCHECK(arena != nullptr); + return absl::visit( + absl::Overload( + [arena](String& string) -> BytesValue { + return BytesValue::From(std::move(string.target), arena); + }, + [arena](Cord& cord) -> BytesValue { + return BytesValue::From(cord.stream.Consume(), arena); + }), + AsVariant()); } private: struct String final { - String(absl::string_view target, google::protobuf::Arena* absl_nullable arena) - : target(target), stream(&this->target), arena(arena) {} + explicit String(absl::string_view target) + : target(target), stream(&this->target) {} std::string target; google::protobuf::io::StringOutputStream stream; - google::protobuf::Arena* absl_nullable arena; }; - using Cord = google::protobuf::io::CordOutputStream; + struct Cord final { + explicit Cord(const absl::Cord& cord) : stream(cord) {} + + google::protobuf::io::CordOutputStream stream; + }; using Variant = absl::variant; - void Construct(const BytesValue& value, google::protobuf::Arena* absl_nullable arena) { + void Construct(const BytesValue& value) { switch (value.value_.GetKind()) { case common_internal::ByteStringKind::kSmall: - Construct(value.value_.GetSmall(), arena); + Construct(value.value_.GetSmall()); break; case common_internal::ByteStringKind::kMedium: - Construct(value.value_.GetMedium(), arena); + Construct(value.value_.GetMedium()); break; case common_internal::ByteStringKind::kLarge: Construct(value.value_.GetLarge()); @@ -148,9 +149,9 @@ class BytesValueOutputStream final : public google::protobuf::io::ZeroCopyOutput } } - void Construct(absl::string_view value, google::protobuf::Arena* absl_nullable arena) { + void Construct(absl::string_view value) { ::new (static_cast(&impl_[0])) - Variant(absl::in_place_type, value, arena); + Variant(absl::in_place_type, value); } void Construct(const absl::Cord& value) { diff --git a/common/values/bytes_value_test.cc b/common/values/bytes_value_test.cc index 367a8ca16..e4e7ad665 100644 --- a/common/values/bytes_value_test.cc +++ b/common/values/bytes_value_test.cc @@ -20,7 +20,6 @@ #include "absl/status/status_matchers.h" #include "absl/strings/cord.h" #include "absl/strings/cord_test_helpers.h" -#include "absl/types/optional.h" #include "common/native_type.h" #include "common/value.h" #include "common/value_testing.h" @@ -38,130 +37,139 @@ using ::testing::Optional; using BytesValueTest = common_internal::ValueTest<>; TEST_F(BytesValueTest, Kind) { - EXPECT_EQ(BytesValue("foo").kind(), BytesValue::kKind); - EXPECT_EQ(Value(BytesValue(absl::Cord("foo"))).kind(), BytesValue::kKind); + EXPECT_EQ(BytesValue::WrapUnsafe("foo").kind(), BytesValue::kKind); + EXPECT_EQ(Value(BytesValue::From(absl::Cord("foo"), arena())).kind(), + BytesValue::kKind); } TEST_F(BytesValueTest, DebugString) { { std::ostringstream out; - out << BytesValue("foo"); + out << BytesValue::WrapUnsafe("foo"); EXPECT_EQ(out.str(), "b\"foo\""); } { std::ostringstream out; - out << BytesValue(absl::MakeFragmentedCord({"f", "o", "o"})); + out << BytesValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()); EXPECT_EQ(out.str(), "b\"foo\""); } { std::ostringstream out; - out << Value(BytesValue(absl::Cord("foo"))); + out << Value(BytesValue::From(absl::Cord("foo"), arena())); EXPECT_EQ(out.str(), "b\"foo\""); } } TEST_F(BytesValueTest, ConvertToJson) { auto* message = NewArenaValueMessage(); - EXPECT_THAT(BytesValue("foo").ConvertToJson(descriptor_pool(), - message_factory(), message), + EXPECT_THAT(BytesValue::WrapUnsafe("foo").ConvertToJson( + descriptor_pool(), message_factory(), message), IsOk()); EXPECT_THAT(*message, EqualsValueTextProto(R"pb(string_value: "Zm9v")pb")); } TEST_F(BytesValueTest, NativeValue) { std::string scratch; - EXPECT_EQ(BytesValue("foo").NativeString(), "foo"); - EXPECT_EQ(BytesValue("foo").NativeString(scratch), "foo"); - EXPECT_EQ(BytesValue("foo").NativeCord(), "foo"); + EXPECT_EQ(BytesValue::WrapUnsafe("foo").NativeString(), "foo"); + EXPECT_EQ(BytesValue::WrapUnsafe("foo").NativeString(scratch), "foo"); + EXPECT_EQ(BytesValue::WrapUnsafe("foo").NativeCord(), "foo"); } TEST_F(BytesValueTest, TryFlat) { - EXPECT_THAT(BytesValue("foo").TryFlat(), Optional(Eq("foo"))); + EXPECT_THAT(BytesValue::WrapUnsafe("foo").TryFlat(), Optional(Eq("foo"))); EXPECT_THAT( - BytesValue(absl::MakeFragmentedCord({"Hello, World!", "World, Hello!"})) + BytesValue::From( + absl::MakeFragmentedCord({"Hello, World!", "World, Hello!"}), arena()) .TryFlat(), Eq(std::nullopt)); } TEST_F(BytesValueTest, ToString) { - EXPECT_EQ(BytesValue("foo").ToString(), "foo"); - EXPECT_EQ(BytesValue(absl::MakeFragmentedCord({"f", "o", "o"})).ToString(), + EXPECT_EQ(BytesValue::WrapUnsafe("foo").ToString(), "foo"); + EXPECT_EQ(BytesValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .ToString(), "foo"); } TEST_F(BytesValueTest, CopyToString) { std::string out; - BytesValue("foo").CopyToString(&out); + BytesValue::WrapUnsafe("foo").CopyToString(&out); EXPECT_EQ(out, "foo"); - BytesValue(absl::MakeFragmentedCord({"f", "o", "o"})).CopyToString(&out); + BytesValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .CopyToString(&out); EXPECT_EQ(out, "foo"); } TEST_F(BytesValueTest, AppendToString) { std::string out; - BytesValue("foo").AppendToString(&out); + BytesValue::WrapUnsafe("foo").AppendToString(&out); EXPECT_EQ(out, "foo"); - BytesValue(absl::MakeFragmentedCord({"f", "o", "o"})).AppendToString(&out); + BytesValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .AppendToString(&out); EXPECT_EQ(out, "foofoo"); } TEST_F(BytesValueTest, ToCord) { - EXPECT_EQ(BytesValue("foo").ToCord(), "foo"); - EXPECT_EQ(BytesValue(absl::MakeFragmentedCord({"f", "o", "o"})).ToCord(), + EXPECT_EQ(BytesValue::WrapUnsafe("foo").ToCord(), "foo"); + EXPECT_EQ(BytesValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .ToCord(), "foo"); } TEST_F(BytesValueTest, CopyToCord) { absl::Cord out; - BytesValue("foo").CopyToCord(&out); + BytesValue::WrapUnsafe("foo").CopyToCord(&out); EXPECT_EQ(out, "foo"); - BytesValue(absl::MakeFragmentedCord({"f", "o", "o"})).CopyToCord(&out); + BytesValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .CopyToCord(&out); EXPECT_EQ(out, "foo"); } TEST_F(BytesValueTest, AppendToCord) { absl::Cord out; - BytesValue("foo").AppendToCord(&out); + BytesValue::WrapUnsafe("foo").AppendToCord(&out); EXPECT_EQ(out, "foo"); - BytesValue(absl::MakeFragmentedCord({"f", "o", "o"})).AppendToCord(&out); + BytesValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .AppendToCord(&out); EXPECT_EQ(out, "foofoo"); } TEST_F(BytesValueTest, NativeTypeId) { - EXPECT_EQ(NativeTypeId::Of(BytesValue("foo")), - NativeTypeId::For()); - EXPECT_EQ(NativeTypeId::Of(Value(BytesValue(absl::Cord("foo")))), + EXPECT_EQ(NativeTypeId::Of(BytesValue::WrapUnsafe("foo")), NativeTypeId::For()); + EXPECT_EQ( + NativeTypeId::Of(Value(BytesValue::From(absl::Cord("foo"), arena()))), + NativeTypeId::For()); } TEST_F(BytesValueTest, StringViewEquality) { // NOLINTBEGIN(readability/check) - EXPECT_TRUE(BytesValue("foo") == "foo"); - EXPECT_FALSE(BytesValue("foo") == "bar"); + EXPECT_TRUE(BytesValue::WrapUnsafe("foo") == "foo"); + EXPECT_FALSE(BytesValue::WrapUnsafe("foo") == "bar"); - EXPECT_TRUE("foo" == BytesValue("foo")); - EXPECT_FALSE("bar" == BytesValue("foo")); + EXPECT_TRUE("foo" == BytesValue::WrapUnsafe("foo")); + EXPECT_FALSE("bar" == BytesValue::WrapUnsafe("foo")); // NOLINTEND(readability/check) } TEST_F(BytesValueTest, StringViewInequality) { // NOLINTBEGIN(readability/check) - EXPECT_FALSE(BytesValue("foo") != "foo"); - EXPECT_TRUE(BytesValue("foo") != "bar"); + EXPECT_FALSE(BytesValue::WrapUnsafe("foo") != "foo"); + EXPECT_TRUE(BytesValue::WrapUnsafe("foo") != "bar"); - EXPECT_FALSE("foo" != BytesValue("foo")); - EXPECT_TRUE("bar" != BytesValue("foo")); + EXPECT_FALSE("foo" != BytesValue::WrapUnsafe("foo")); + EXPECT_TRUE("bar" != BytesValue::WrapUnsafe("foo")); // NOLINTEND(readability/check) } TEST_F(BytesValueTest, Comparison) { - EXPECT_LT(BytesValue("bar"), BytesValue("foo")); - EXPECT_FALSE(BytesValue("foo") < BytesValue("foo")); - EXPECT_FALSE(BytesValue("foo") < BytesValue("bar")); + EXPECT_LT(BytesValue::WrapUnsafe("bar"), BytesValue::WrapUnsafe("foo")); + EXPECT_FALSE(BytesValue::WrapUnsafe("foo") < BytesValue::WrapUnsafe("foo")); + EXPECT_FALSE(BytesValue::WrapUnsafe("foo") < BytesValue::WrapUnsafe("bar")); } TEST_F(BytesValueTest, StringInputStream) { - BytesValue value = BytesValue("foo"); + BytesValue value = BytesValue::WrapUnsafe("foo"); BytesValueInputStream stream(&value); const void* data; int size; @@ -177,7 +185,7 @@ TEST_F(BytesValueTest, StringInputStream) { } TEST_F(BytesValueTest, CordInputStream) { - BytesValue value = BytesValue(absl::Cord("foo")); + BytesValue value = BytesValue::From(absl::Cord("foo"), arena()); BytesValueInputStream stream(&value); const void* data; int size; @@ -193,9 +201,9 @@ TEST_F(BytesValueTest, CordInputStream) { } TEST_F(BytesValueTest, ArenaStringOutputStream) { - BytesValue value = BytesValue(""); + BytesValue value = BytesValue(); { - BytesValueOutputStream stream(value, arena()); + BytesValueOutputStream stream(value); EXPECT_THAT(stream.AllowsAliasing(), An()); EXPECT_EQ(stream.ByteCount(), 0); google::protobuf::Value value_proto; @@ -203,17 +211,17 @@ TEST_F(BytesValueTest, ArenaStringOutputStream) { (*struct_proto->mutable_fields())["foo"].set_string_value("bar"); (*struct_proto->mutable_fields())["baz"].set_number_value(3.14159); ASSERT_TRUE(value_proto.SerializePartialToZeroCopyStream(&stream)); - EXPECT_EQ(std::move(stream).Consume(), + EXPECT_EQ(std::move(stream).Consume(arena()), value_proto.SerializePartialAsString()); } { BytesValueOutputStream stream(value); - EXPECT_EQ(std::move(stream).Consume(), ""); + EXPECT_EQ(std::move(stream).Consume(arena()), ""); } } TEST_F(BytesValueTest, StringOutputStream) { - BytesValue value = BytesValue(""); + BytesValue value = BytesValue(); { BytesValueOutputStream stream(value); EXPECT_THAT(stream.AllowsAliasing(), An()); @@ -223,17 +231,18 @@ TEST_F(BytesValueTest, StringOutputStream) { (*struct_proto->mutable_fields())["foo"].set_string_value("bar"); (*struct_proto->mutable_fields())["baz"].set_number_value(3.14159); ASSERT_TRUE(value_proto.SerializePartialToZeroCopyStream(&stream)); - EXPECT_EQ(std::move(stream).Consume(), + EXPECT_EQ(std::move(stream).Consume(arena()), value_proto.SerializePartialAsString()); } { BytesValueOutputStream stream(value); - EXPECT_EQ(std::move(stream).Consume(), ""); + EXPECT_EQ(std::move(stream).Consume(arena()), ""); } } TEST_F(BytesValueTest, CordOutputStream) { - BytesValue value = BytesValue(absl::Cord()); + absl::Cord cord; + BytesValue value = BytesValue::WrapUnsafe(&cord); { BytesValueOutputStream stream(value); EXPECT_THAT(stream.AllowsAliasing(), An()); @@ -243,12 +252,12 @@ TEST_F(BytesValueTest, CordOutputStream) { (*struct_proto->mutable_fields())["foo"].set_string_value("bar"); (*struct_proto->mutable_fields())["baz"].set_number_value(3.14159); ASSERT_TRUE(value_proto.SerializePartialToZeroCopyStream(&stream)); - EXPECT_EQ(std::move(stream).Consume(), + EXPECT_EQ(std::move(stream).Consume(arena()), value_proto.SerializePartialAsString()); } { BytesValueOutputStream stream(value); - EXPECT_EQ(std::move(stream).Consume(), ""); + EXPECT_EQ(std::move(stream).Consume(arena()), ""); } } diff --git a/common/values/custom_list_value.cc b/common/values/custom_list_value.cc index fbba38cfa..ca001b103 100644 --- a/common/values/custom_list_value.cc +++ b/common/values/custom_list_value.cc @@ -486,10 +486,7 @@ CustomListValue CustomListValue::Clone( CustomListValueInterface::Content content = content_.To(); ABSL_DCHECK(content.interface != nullptr); - if (content.arena != arena) { - return content.interface->Clone(arena); - } - return *this; + return content.interface->Clone(arena); } return dispatcher_->clone(dispatcher_, content_, arena); } diff --git a/common/values/custom_map_value.cc b/common/values/custom_map_value.cc index ecd04abfd..e3d1cf3ee 100644 --- a/common/values/custom_map_value.cc +++ b/common/values/custom_map_value.cc @@ -589,10 +589,7 @@ CustomMapValue CustomMapValue::Clone(google::protobuf::Arena* absl_nonnull arena CustomMapValueInterface::Content content = content_.To(); ABSL_DCHECK(content.interface != nullptr); - if (content.arena != arena) { - return content.interface->Clone(arena); - } - return *this; + return content.interface->Clone(arena); } return dispatcher_->clone(dispatcher_, content_, arena); } diff --git a/common/values/custom_map_value_test.cc b/common/values/custom_map_value_test.cc index 11c46d4cf..2eb833ac0 100644 --- a/common/values/custom_map_value_test.cc +++ b/common/values/custom_map_value_test.cc @@ -109,8 +109,8 @@ class CustomMapValueInterfaceTest final : public CustomMapValueInterface { ListValue* absl_nonnull result) const override { auto builder = common_internal::NewListValueBuilder(arena); builder->Reserve(2); - CEL_RETURN_IF_ERROR(builder->Add(StringValue("foo"))); - CEL_RETURN_IF_ERROR(builder->Add(StringValue("bar"))); + CEL_RETURN_IF_ERROR(builder->Add(StringValue::WrapUnsafe("foo"))); + CEL_RETURN_IF_ERROR(builder->Add(StringValue::WrapUnsafe("bar"))); *result = std::move(*builder).Build(); return absl::OkStatus(); } @@ -293,8 +293,8 @@ class CustomMapValueTest : public common_internal::ValueTest<> { ListValue* absl_nonnull result) -> absl::Status { auto builder = common_internal::NewListValueBuilder(arena); builder->Reserve(2); - CEL_RETURN_IF_ERROR(builder->Add(StringValue("foo"))); - CEL_RETURN_IF_ERROR(builder->Add(StringValue("bar"))); + CEL_RETURN_IF_ERROR(builder->Add(StringValue::WrapUnsafe("foo"))); + CEL_RETURN_IF_ERROR(builder->Add(StringValue::WrapUnsafe("bar"))); *result = std::move(*builder).Build(); return absl::OkStatus(); }, @@ -456,76 +456,76 @@ TEST_F(CustomMapValueTest, Interface_Size) { TEST_F(CustomMapValueTest, Dispatcher_Get) { CustomMapValue map = MakeDispatcher(); - ASSERT_THAT(map.Get(StringValue("foo"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Get(StringValue::WrapUnsafe("foo"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - ASSERT_THAT(map.Get(StringValue("bar"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Get(StringValue::WrapUnsafe("bar"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(IntValueIs(1))); ASSERT_THAT( - map.Get(StringValue("baz"), descriptor_pool(), message_factory(), - arena()), + map.Get(StringValue::WrapUnsafe("baz"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(ErrorValueIs(StatusIs(absl::StatusCode::kNotFound)))); } TEST_F(CustomMapValueTest, Interface_Get) { CustomMapValue map = MakeInterface(); - ASSERT_THAT(map.Get(StringValue("foo"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Get(StringValue::WrapUnsafe("foo"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - ASSERT_THAT(map.Get(StringValue("bar"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Get(StringValue::WrapUnsafe("bar"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(IntValueIs(1))); ASSERT_THAT( - map.Get(StringValue("baz"), descriptor_pool(), message_factory(), - arena()), + map.Get(StringValue::WrapUnsafe("baz"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(ErrorValueIs(StatusIs(absl::StatusCode::kNotFound)))); } TEST_F(CustomMapValueTest, Dispatcher_Find) { CustomMapValue map = MakeDispatcher(); - ASSERT_THAT(map.Find(StringValue("foo"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Find(StringValue::WrapUnsafe("foo"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(BoolValueIs(true)))); - ASSERT_THAT(map.Find(StringValue("bar"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Find(StringValue::WrapUnsafe("bar"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IntValueIs(1)))); - ASSERT_THAT(map.Find(StringValue("baz"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Find(StringValue::WrapUnsafe("baz"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Eq(std::nullopt))); } TEST_F(CustomMapValueTest, Interface_Find) { CustomMapValue map = MakeInterface(); - ASSERT_THAT(map.Find(StringValue("foo"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Find(StringValue::WrapUnsafe("foo"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(BoolValueIs(true)))); - ASSERT_THAT(map.Find(StringValue("bar"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Find(StringValue::WrapUnsafe("bar"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Optional(IntValueIs(1)))); - ASSERT_THAT(map.Find(StringValue("baz"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Find(StringValue::WrapUnsafe("baz"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(Eq(std::nullopt))); } TEST_F(CustomMapValueTest, Dispatcher_Find_Error) { CustomMapValue map = MakeDispatcher(); Value result; - ASSERT_THAT(map.Find(StringValue("error"), descriptor_pool(), + ASSERT_THAT(map.Find(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena(), &result), IsOkAndHolds(false)); EXPECT_THAT(result, ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument, "custom error"))); - ASSERT_THAT(map.Get(StringValue("error"), descriptor_pool(), + ASSERT_THAT(map.Get(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena(), &result), IsOk()); EXPECT_THAT(result, ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument, "custom error"))); - EXPECT_THAT(map.Get(StringValue("error"), descriptor_pool(), + EXPECT_THAT(map.Get(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(ErrorValueIs(StatusIs( absl::StatusCode::kInvalidArgument, "custom error")))); - EXPECT_THAT(map.Find(StringValue("error"), descriptor_pool(), + EXPECT_THAT(map.Find(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(Eq(std::nullopt))); } @@ -533,21 +533,21 @@ TEST_F(CustomMapValueTest, Dispatcher_Find_Error) { TEST_F(CustomMapValueTest, Interface_Find_Error) { CustomMapValue map = MakeInterface(); Value result; - ASSERT_THAT(map.Find(StringValue("error"), descriptor_pool(), + ASSERT_THAT(map.Find(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena(), &result), IsOkAndHolds(false)); EXPECT_THAT(result, ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument, "custom error"))); - ASSERT_THAT(map.Get(StringValue("error"), descriptor_pool(), + ASSERT_THAT(map.Get(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena(), &result), IsOk()); EXPECT_THAT(result, ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument, "custom error"))); - EXPECT_THAT(map.Get(StringValue("error"), descriptor_pool(), + EXPECT_THAT(map.Get(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(ErrorValueIs(StatusIs( absl::StatusCode::kInvalidArgument, "custom error")))); - EXPECT_THAT(map.Find(StringValue("error"), descriptor_pool(), + EXPECT_THAT(map.Find(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(Eq(std::nullopt))); } @@ -654,39 +654,39 @@ TEST_F(CustomMapValueTest, Interface_Find_SpecialKeys) { TEST_F(CustomMapValueTest, Dispatcher_Has) { CustomMapValue map = MakeDispatcher(); - ASSERT_THAT(map.Has(StringValue("foo"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Has(StringValue::WrapUnsafe("foo"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - ASSERT_THAT(map.Has(StringValue("bar"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Has(StringValue::WrapUnsafe("bar"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - ASSERT_THAT(map.Has(StringValue("baz"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Has(StringValue::WrapUnsafe("baz"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); } TEST_F(CustomMapValueTest, Interface_Has) { CustomMapValue map = MakeInterface(); - ASSERT_THAT(map.Has(StringValue("foo"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Has(StringValue::WrapUnsafe("foo"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - ASSERT_THAT(map.Has(StringValue("bar"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Has(StringValue::WrapUnsafe("bar"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - ASSERT_THAT(map.Has(StringValue("baz"), descriptor_pool(), message_factory(), - arena()), + ASSERT_THAT(map.Has(StringValue::WrapUnsafe("baz"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); } TEST_F(CustomMapValueTest, Dispatcher_Has_Error) { CustomMapValue map = MakeDispatcher(); Value result; - ASSERT_THAT(map.Has(StringValue("error"), descriptor_pool(), + ASSERT_THAT(map.Has(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena(), &result), IsOk()); EXPECT_THAT(result, ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument, "custom error"))); - EXPECT_THAT(map.Has(StringValue("error"), descriptor_pool(), + EXPECT_THAT(map.Has(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(ErrorValueIs(StatusIs( absl::StatusCode::kInvalidArgument, "custom error")))); @@ -695,12 +695,12 @@ TEST_F(CustomMapValueTest, Dispatcher_Has_Error) { TEST_F(CustomMapValueTest, Interface_Has_Error) { CustomMapValue map = MakeInterface(); Value result; - ASSERT_THAT(map.Has(StringValue("error"), descriptor_pool(), + ASSERT_THAT(map.Has(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena(), &result), IsOk()); EXPECT_THAT(result, ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument, "custom error"))); - EXPECT_THAT(map.Has(StringValue("error"), descriptor_pool(), + EXPECT_THAT(map.Has(StringValue::WrapUnsafe("error"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(ErrorValueIs(StatusIs( absl::StatusCode::kInvalidArgument, "custom error")))); diff --git a/common/values/legacy_struct_value_test.cc b/common/values/legacy_struct_value_test.cc index 2e1949c0b..1329232f8 100644 --- a/common/values/legacy_struct_value_test.cc +++ b/common/values/legacy_struct_value_test.cc @@ -354,8 +354,8 @@ TEST_F(LegacyStructValueTest, WrapLegacyFieldAccessResultParsedJsonList) { auto list_val = val.GetList(); EXPECT_THAT(list_val.IsEmpty(), IsOkAndHolds(false)); EXPECT_THAT(list_val.Size(), IsOkAndHolds(2)); - EXPECT_THAT(list_val.Contains(StringValue("item1"), descriptor_pool(), - message_factory(), arena()), + EXPECT_THAT(list_val.Contains(StringValue::WrapUnsafe("item1"), + descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); Value elem; @@ -403,10 +403,10 @@ TEST_F(LegacyStructValueTest, WrapLegacyFieldAccessResultParsedMapField) { EXPECT_THAT(map_val.IsEmpty(), IsOkAndHolds(false)); EXPECT_FALSE(map_val.IsZeroValue()); EXPECT_THAT(map_val.Size(), IsOkAndHolds(2)); - EXPECT_THAT(map_val.Has(StringValue("key1"), descriptor_pool(), + EXPECT_THAT(map_val.Has(StringValue::WrapUnsafe("key1"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - EXPECT_THAT(map_val.Has(StringValue("missing"), descriptor_pool(), + EXPECT_THAT(map_val.Has(StringValue::WrapUnsafe("missing"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); EXPECT_THAT( @@ -414,13 +414,13 @@ TEST_F(LegacyStructValueTest, WrapLegacyFieldAccessResultParsedMapField) { IsOkAndHolds(ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument)))); Value found_val; - ASSERT_THAT(map_val.Find(StringValue("key1"), descriptor_pool(), + ASSERT_THAT(map_val.Find(StringValue::WrapUnsafe("key1"), descriptor_pool(), message_factory(), arena(), &found_val), IsOkAndHolds(true)); EXPECT_THAT(found_val, StringValueIs("val1")); Value get_val; - ASSERT_THAT(map_val.Get(StringValue("key2"), descriptor_pool(), + ASSERT_THAT(map_val.Get(StringValue::WrapUnsafe("key2"), descriptor_pool(), message_factory(), arena(), &get_val), IsOk()); EXPECT_THAT(get_val, StringValueIs("val2")); @@ -466,10 +466,10 @@ TEST_F(LegacyStructValueTest, WrapLegacyFieldAccessResultParsedJsonMap) { EXPECT_THAT(map_val.IsEmpty(), IsOkAndHolds(false)); EXPECT_FALSE(map_val.IsZeroValue()); EXPECT_THAT(map_val.Size(), IsOkAndHolds(2)); - EXPECT_THAT(map_val.Has(StringValue("k1"), descriptor_pool(), + EXPECT_THAT(map_val.Has(StringValue::WrapUnsafe("k1"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - EXPECT_THAT(map_val.Has(StringValue("missing"), descriptor_pool(), + EXPECT_THAT(map_val.Has(StringValue::WrapUnsafe("missing"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); EXPECT_THAT( @@ -477,13 +477,13 @@ TEST_F(LegacyStructValueTest, WrapLegacyFieldAccessResultParsedJsonMap) { IsOkAndHolds(ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument)))); Value found_val; - ASSERT_THAT(map_val.Find(StringValue("k1"), descriptor_pool(), + ASSERT_THAT(map_val.Find(StringValue::WrapUnsafe("k1"), descriptor_pool(), message_factory(), arena(), &found_val), IsOkAndHolds(true)); EXPECT_THAT(found_val, StringValueIs("v1")); Value get_val; - ASSERT_THAT(map_val.Get(StringValue("k2"), descriptor_pool(), + ASSERT_THAT(map_val.Get(StringValue::WrapUnsafe("k2"), descriptor_pool(), message_factory(), arena(), &get_val), IsOk()); EXPECT_THAT(get_val, StringValueIs("v2")); diff --git a/common/values/map_value_test.cc b/common/values/map_value_test.cc index f7d1c5197..a1a82fd03 100644 --- a/common/values/map_value_test.cc +++ b/common/values/map_value_test.cc @@ -270,9 +270,10 @@ TEST_F(MapValueTest, NewIterator) { TEST_F(MapValueTest, ConvertToJson) { ASSERT_OK_AND_ASSIGN( auto value, - NewJsonMapValue(std::pair{StringValue("0"), DoubleValue(3.0)}, - std::pair{StringValue("1"), DoubleValue(4.0)}, - std::pair{StringValue("2"), DoubleValue(5.0)})); + NewJsonMapValue( + std::pair{StringValue::WrapUnsafe("0"), DoubleValue(3.0)}, + std::pair{StringValue::WrapUnsafe("1"), DoubleValue(4.0)}, + std::pair{StringValue::WrapUnsafe("2"), DoubleValue(5.0)})); auto* message = NewArenaValueMessage(); EXPECT_THAT( value.ConvertToJson(descriptor_pool(), message_factory(), message), diff --git a/common/values/mutable_list_value_test.cc b/common/values/mutable_list_value_test.cc index c08d7091c..c6aec0df4 100644 --- a/common/values/mutable_list_value_test.cc +++ b/common/values/mutable_list_value_test.cc @@ -47,7 +47,8 @@ TEST_F(MutableListValueTest, IsEmpty) { auto* mutable_list_value = NewMutableListValue(arena()); mutable_list_value->Reserve(1); EXPECT_TRUE(CustomListValue(mutable_list_value, arena()).IsEmpty()); - EXPECT_THAT(mutable_list_value->Append(StringValue("foo")), IsOk()); + EXPECT_THAT(mutable_list_value->Append(StringValue::WrapUnsafe("foo")), + IsOk()); EXPECT_FALSE(CustomListValue(mutable_list_value, arena()).IsEmpty()); } @@ -55,7 +56,8 @@ TEST_F(MutableListValueTest, Size) { auto* mutable_list_value = NewMutableListValue(arena()); mutable_list_value->Reserve(1); EXPECT_THAT(CustomListValue(mutable_list_value, arena()).Size(), 0); - EXPECT_THAT(mutable_list_value->Append(StringValue("foo")), IsOk()); + EXPECT_THAT(mutable_list_value->Append(StringValue::WrapUnsafe("foo")), + IsOk()); EXPECT_THAT(CustomListValue(mutable_list_value, arena()).Size(), 1); } @@ -73,7 +75,8 @@ TEST_F(MutableListValueTest, ForEach) { message_factory(), arena()), IsOk()); EXPECT_THAT(elements, IsEmpty()); - EXPECT_THAT(mutable_list_value->Append(StringValue("foo")), IsOk()); + EXPECT_THAT(mutable_list_value->Append(StringValue::WrapUnsafe("foo")), + IsOk()); EXPECT_THAT(CustomListValue(mutable_list_value, arena()) .ForEach(for_each_callback, descriptor_pool(), message_factory(), arena()), @@ -89,7 +92,8 @@ TEST_F(MutableListValueTest, NewIterator) { CustomListValue(mutable_list_value, arena()).NewIterator()); EXPECT_THAT(iterator->Next(descriptor_pool(), message_factory(), arena()), StatusIs(absl::StatusCode::kFailedPrecondition)); - EXPECT_THAT(mutable_list_value->Append(StringValue("foo")), IsOk()); + EXPECT_THAT(mutable_list_value->Append(StringValue::WrapUnsafe("foo")), + IsOk()); ASSERT_OK_AND_ASSIGN( iterator, CustomListValue(mutable_list_value, arena()).NewIterator()); EXPECT_TRUE(iterator->HasNext()); @@ -110,7 +114,8 @@ TEST_F(MutableListValueTest, Get) { IsOk()); EXPECT_THAT(value, ErrorValueIs(StatusIs(absl::StatusCode::kInvalidArgument))); - EXPECT_THAT(mutable_list_value->Append(StringValue("foo")), IsOk()); + EXPECT_THAT(mutable_list_value->Append(StringValue::WrapUnsafe("foo")), + IsOk()); EXPECT_THAT( CustomListValue(mutable_list_value, arena()) .Get(0, descriptor_pool(), message_factory(), arena(), &value), diff --git a/common/values/mutable_map_value_test.cc b/common/values/mutable_map_value_test.cc index 2f08abe3f..c03e9cbd5 100644 --- a/common/values/mutable_map_value_test.cc +++ b/common/values/mutable_map_value_test.cc @@ -52,7 +52,9 @@ TEST_F(MutableMapValueTest, IsEmpty) { auto mutable_map_value = NewMutableMapValue(arena()); mutable_map_value->Reserve(1); EXPECT_TRUE(CustomMapValue(mutable_map_value, arena()).IsEmpty()); - EXPECT_THAT(mutable_map_value->Put(StringValue("foo"), IntValue(1)), IsOk()); + EXPECT_THAT( + mutable_map_value->Put(StringValue::WrapUnsafe("foo"), IntValue(1)), + IsOk()); EXPECT_FALSE(CustomMapValue(mutable_map_value, arena()).IsEmpty()); } @@ -60,7 +62,9 @@ TEST_F(MutableMapValueTest, Size) { auto mutable_map_value = NewMutableMapValue(arena()); mutable_map_value->Reserve(1); EXPECT_THAT(CustomMapValue(mutable_map_value, arena()).Size(), 0); - EXPECT_THAT(mutable_map_value->Put(StringValue("foo"), IntValue(1)), IsOk()); + EXPECT_THAT( + mutable_map_value->Put(StringValue::WrapUnsafe("foo"), IntValue(1)), + IsOk()); EXPECT_THAT(CustomMapValue(mutable_map_value, arena()).Size(), 1); } @@ -68,7 +72,9 @@ TEST_F(MutableMapValueTest, ListKeys) { auto mutable_map_value = NewMutableMapValue(arena()); mutable_map_value->Reserve(1); ListValue keys; - EXPECT_THAT(mutable_map_value->Put(StringValue("foo"), IntValue(1)), IsOk()); + EXPECT_THAT( + mutable_map_value->Put(StringValue::WrapUnsafe("foo"), IntValue(1)), + IsOk()); EXPECT_THAT( CustomMapValue(mutable_map_value, arena()) .ListKeys(descriptor_pool(), message_factory(), arena(), &keys), @@ -92,7 +98,9 @@ TEST_F(MutableMapValueTest, ForEach) { message_factory(), arena()), IsOk()); EXPECT_THAT(entries, IsEmpty()); - EXPECT_THAT(mutable_map_value->Put(StringValue("foo"), IntValue(1)), IsOk()); + EXPECT_THAT( + mutable_map_value->Put(StringValue::WrapUnsafe("foo"), IntValue(1)), + IsOk()); EXPECT_THAT(CustomMapValue(mutable_map_value, arena()) .ForEach(for_each_callback, descriptor_pool(), message_factory(), arena()), @@ -109,7 +117,9 @@ TEST_F(MutableMapValueTest, NewIterator) { EXPECT_FALSE(iterator->HasNext()); EXPECT_THAT(iterator->Next(descriptor_pool(), message_factory(), arena()), StatusIs(absl::StatusCode::kFailedPrecondition)); - EXPECT_THAT(mutable_map_value->Put(StringValue("foo"), IntValue(1)), IsOk()); + EXPECT_THAT( + mutable_map_value->Put(StringValue::WrapUnsafe("foo"), IntValue(1)), + IsOk()); ASSERT_OK_AND_ASSIGN( iterator, CustomMapValue(mutable_map_value, arena()).NewIterator()); EXPECT_TRUE(iterator->HasNext()); @@ -125,24 +135,26 @@ TEST_F(MutableMapValueTest, FindHas) { mutable_map_value->Reserve(1); Value value; EXPECT_THAT(CustomMapValue(mutable_map_value, arena()) - .Find(StringValue("foo"), descriptor_pool(), + .Find(StringValue::WrapUnsafe("foo"), descriptor_pool(), message_factory(), arena(), &value), IsOkAndHolds(IsFalse())); EXPECT_THAT(value, IsNullValue()); EXPECT_THAT(CustomMapValue(mutable_map_value, arena()) - .Has(StringValue("foo"), descriptor_pool(), message_factory(), - arena(), &value), + .Has(StringValue::WrapUnsafe("foo"), descriptor_pool(), + message_factory(), arena(), &value), IsOk()); EXPECT_THAT(value, BoolValueIs(false)); - EXPECT_THAT(mutable_map_value->Put(StringValue("foo"), IntValue(1)), IsOk()); + EXPECT_THAT( + mutable_map_value->Put(StringValue::WrapUnsafe("foo"), IntValue(1)), + IsOk()); EXPECT_THAT(CustomMapValue(mutable_map_value, arena()) - .Find(StringValue("foo"), descriptor_pool(), + .Find(StringValue::WrapUnsafe("foo"), descriptor_pool(), message_factory(), arena(), &value), IsOkAndHolds(IsTrue())); EXPECT_THAT(value, IntValueIs(1)); EXPECT_THAT(CustomMapValue(mutable_map_value, arena()) - .Has(StringValue("foo"), descriptor_pool(), message_factory(), - arena(), &value), + .Has(StringValue::WrapUnsafe("foo"), descriptor_pool(), + message_factory(), arena(), &value), IsOk()); EXPECT_THAT(value, BoolValueIs(true)); } diff --git a/common/values/parsed_json_list_value_test.cc b/common/values/parsed_json_list_value_test.cc index 5fcff20bf..7551463f4 100644 --- a/common/values/parsed_json_list_value_test.cc +++ b/common/values/parsed_json_list_value_test.cc @@ -251,12 +251,14 @@ TEST_F(ParsedJsonListValueTest, Contains_Dynamic) { EXPECT_THAT(valid_value.Contains(DoubleValue(1.0), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - EXPECT_THAT(valid_value.Contains(StringValue("bar"), descriptor_pool(), - message_factory(), arena()), - IsOkAndHolds(BoolValueIs(false))); - EXPECT_THAT(valid_value.Contains(StringValue("foo"), descriptor_pool(), - message_factory(), arena()), - IsOkAndHolds(BoolValueIs(true))); + EXPECT_THAT( + valid_value.Contains(StringValue::WrapUnsafe("bar"), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(BoolValueIs(false))); + EXPECT_THAT( + valid_value.Contains(StringValue::WrapUnsafe("foo"), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(BoolValueIs(true))); EXPECT_THAT(valid_value.Contains( ParsedJsonListValue( DynamicParseTextProto( diff --git a/common/values/parsed_json_map_value.cc b/common/values/parsed_json_map_value.cc index ec8c91a4f..ad0ad558c 100644 --- a/common/values/parsed_json_map_value.cc +++ b/common/values/parsed_json_map_value.cc @@ -325,7 +325,7 @@ absl::Status ParsedJsonMapValue::ForEach( const auto map_end = reflection.EndFields(*value_); for (; map_begin != map_end; ++map_begin) { // We have to copy until `google::protobuf::MapKey` is just a view. - key_scratch = StringValue(arena, map_begin.GetKey().GetStringValue()); + key_scratch = StringValue::From(map_begin.GetKey().GetStringValue(), arena); value_scratch = common_internal::ParsedJsonValue( &map_begin.GetValueRef().GetMessageValue(), arena); CEL_ASSIGN_OR_RETURN(auto ok, callback(key_scratch, value_scratch)); diff --git a/common/values/parsed_json_map_value_test.cc b/common/values/parsed_json_map_value_test.cc index 67bddcf8e..8dfca0c70 100644 --- a/common/values/parsed_json_map_value_test.cc +++ b/common/values/parsed_json_map_value_test.cc @@ -144,15 +144,15 @@ TEST_F(ParsedJsonMapValueTest, Get_Dynamic) { valid_value.Get(BoolValue(), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(ErrorValueIs(StatusIs(absl::StatusCode::kNotFound)))); - EXPECT_THAT(valid_value.Get(StringValue("foo"), descriptor_pool(), + EXPECT_THAT(valid_value.Get(StringValue::WrapUnsafe("foo"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(IsNullValue())); - EXPECT_THAT(valid_value.Get(StringValue("bar"), descriptor_pool(), + EXPECT_THAT(valid_value.Get(StringValue::WrapUnsafe("bar"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); EXPECT_THAT( - valid_value.Get(StringValue("baz"), descriptor_pool(), message_factory(), - arena()), + valid_value.Get(StringValue::WrapUnsafe("baz"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(ErrorValueIs(StatusIs(absl::StatusCode::kNotFound)))); } @@ -171,14 +171,14 @@ TEST_F(ParsedJsonMapValueTest, Find_Dynamic) { EXPECT_THAT(valid_value.Find(BoolValue(), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(Eq(std::nullopt))); - EXPECT_THAT(valid_value.Find(StringValue("foo"), descriptor_pool(), - message_factory(), arena()), + EXPECT_THAT(valid_value.Find(StringValue::WrapUnsafe("foo"), + descriptor_pool(), message_factory(), arena()), IsOkAndHolds(Optional(IsNullValue()))); - EXPECT_THAT(valid_value.Find(StringValue("bar"), descriptor_pool(), - message_factory(), arena()), + EXPECT_THAT(valid_value.Find(StringValue::WrapUnsafe("bar"), + descriptor_pool(), message_factory(), arena()), IsOkAndHolds(Optional(BoolValueIs(true)))); - EXPECT_THAT(valid_value.Find(StringValue("baz"), descriptor_pool(), - message_factory(), arena()), + EXPECT_THAT(valid_value.Find(StringValue::WrapUnsafe("baz"), + descriptor_pool(), message_factory(), arena()), IsOkAndHolds(Eq(std::nullopt))); } @@ -197,13 +197,13 @@ TEST_F(ParsedJsonMapValueTest, Has_Dynamic) { EXPECT_THAT(valid_value.Has(BoolValue(), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); - EXPECT_THAT(valid_value.Has(StringValue("foo"), descriptor_pool(), + EXPECT_THAT(valid_value.Has(StringValue::WrapUnsafe("foo"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - EXPECT_THAT(valid_value.Has(StringValue("bar"), descriptor_pool(), + EXPECT_THAT(valid_value.Has(StringValue::WrapUnsafe("bar"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - EXPECT_THAT(valid_value.Has(StringValue("baz"), descriptor_pool(), + EXPECT_THAT(valid_value.Has(StringValue::WrapUnsafe("baz"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); } @@ -229,7 +229,7 @@ TEST_F(ParsedJsonMapValueTest, ListKeys_Dynamic) { EXPECT_THAT( keys.Contains(BoolValue(), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); - EXPECT_THAT(keys.Contains(StringValue("bar"), descriptor_pool(), + EXPECT_THAT(keys.Contains(StringValue::WrapUnsafe("bar"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); EXPECT_THAT(keys.Get(0, descriptor_pool(), message_factory(), arena()), @@ -379,10 +379,10 @@ TEST_F(ParsedJsonMapValueTest, CloneDifferentArena) { cloned.Equal(value, descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); EXPECT_EQ(cloned.Size(), 2); - EXPECT_THAT(cloned.Get(StringValue("foo"), descriptor_pool(), + EXPECT_THAT(cloned.Get(StringValue::WrapUnsafe("foo"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(IsNullValue())); - EXPECT_THAT(cloned.Get(StringValue("bar"), descriptor_pool(), + EXPECT_THAT(cloned.Get(StringValue::WrapUnsafe("bar"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); } diff --git a/common/values/parsed_json_value.cc b/common/values/parsed_json_value.cc index 6b10bea40..7c8b72cb6 100644 --- a/common/values/parsed_json_value.cc +++ b/common/values/parsed_json_value.cc @@ -25,8 +25,6 @@ #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "absl/types/variant.h" -#include "common/allocator.h" -#include "common/memory.h" #include "common/value.h" #include "internal/well_known_types.h" #include "google/protobuf/arena.h" @@ -74,17 +72,20 @@ Value ParsedJsonValue(const google::protobuf::Message* absl_nonnull message, } if (string.data() == scratch.data() && string.size() == scratch.size()) { - return StringValue(arena, std::move(scratch)); + return StringValue::From(std::move(scratch), arena); } else { - return StringValue( - Borrower::Arena(MessageArenaOr(message, arena)), string); + if (google::protobuf::Arena* message_arena = message->GetArena(); + message_arena != nullptr) { + return StringValue::Wrap(string, message_arena); + } + return StringValue::From(string, arena); } }, [&](absl::Cord&& cord) -> StringValue { if (cord.empty()) { return StringValue(); } - return StringValue(std::move(cord)); + return StringValue::From(std::move(cord), arena); }), AsVariant(reflection.GetStringValue(*message, scratch))); } diff --git a/common/values/parsed_map_field_value_test.cc b/common/values/parsed_map_field_value_test.cc index 0fdea333b..f605b5c98 100644 --- a/common/values/parsed_map_field_value_test.cc +++ b/common/values/parsed_map_field_value_test.cc @@ -210,15 +210,15 @@ TEST_F(ParsedMapFieldValueTest, Get) { EXPECT_THAT( value.Get(BoolValue(), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(ErrorValueIs(StatusIs(absl::StatusCode::kNotFound)))); - EXPECT_THAT(value.Get(StringValue("foo"), descriptor_pool(), + EXPECT_THAT(value.Get(StringValue::WrapUnsafe("foo"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); - EXPECT_THAT(value.Get(StringValue("bar"), descriptor_pool(), + EXPECT_THAT(value.Get(StringValue::WrapUnsafe("bar"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); EXPECT_THAT( - value.Get(StringValue("baz"), descriptor_pool(), message_factory(), - arena()), + value.Get(StringValue::WrapUnsafe("baz"), descriptor_pool(), + message_factory(), arena()), IsOkAndHolds(ErrorValueIs(StatusIs(absl::StatusCode::kNotFound)))); } @@ -232,13 +232,13 @@ TEST_F(ParsedMapFieldValueTest, Find) { EXPECT_THAT( value.Find(BoolValue(), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(Eq(std::nullopt))); - EXPECT_THAT(value.Find(StringValue("foo"), descriptor_pool(), + EXPECT_THAT(value.Find(StringValue::WrapUnsafe("foo"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(Optional(BoolValueIs(false)))); - EXPECT_THAT(value.Find(StringValue("bar"), descriptor_pool(), + EXPECT_THAT(value.Find(StringValue::WrapUnsafe("bar"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(Optional(BoolValueIs(true)))); - EXPECT_THAT(value.Find(StringValue("baz"), descriptor_pool(), + EXPECT_THAT(value.Find(StringValue::WrapUnsafe("baz"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(Eq(std::nullopt))); } @@ -325,13 +325,13 @@ TEST_F(ParsedMapFieldValueTest, Has) { EXPECT_THAT( value.Has(BoolValue(), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); - EXPECT_THAT(value.Has(StringValue("foo"), descriptor_pool(), + EXPECT_THAT(value.Has(StringValue::WrapUnsafe("foo"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - EXPECT_THAT(value.Has(StringValue("bar"), descriptor_pool(), + EXPECT_THAT(value.Has(StringValue::WrapUnsafe("bar"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); - EXPECT_THAT(value.Has(StringValue("baz"), descriptor_pool(), + EXPECT_THAT(value.Has(StringValue::WrapUnsafe("baz"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); } @@ -351,7 +351,7 @@ TEST_F(ParsedMapFieldValueTest, ListKeys) { EXPECT_THAT( keys.Contains(BoolValue(), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); - EXPECT_THAT(keys.Contains(StringValue("bar"), descriptor_pool(), + EXPECT_THAT(keys.Contains(StringValue::WrapUnsafe("bar"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); EXPECT_THAT(keys.Get(0, descriptor_pool(), message_factory(), arena()), @@ -691,10 +691,10 @@ TEST_F(ParsedMapFieldValueTest, CloneDifferentArena) { cloned.Equal(value, descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(true))); EXPECT_EQ(cloned.Size(), 2); - EXPECT_THAT(cloned.Get(StringValue("foo"), descriptor_pool(), + EXPECT_THAT(cloned.Get(StringValue::WrapUnsafe("foo"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(StringValueIs("bar"))); - EXPECT_THAT(cloned.Get(StringValue("bar"), descriptor_pool(), + EXPECT_THAT(cloned.Get(StringValue::WrapUnsafe("bar"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(StringValueIs("foo"))); } diff --git a/common/values/parsed_repeated_field_value_test.cc b/common/values/parsed_repeated_field_value_test.cc index a3a80456a..bb50bee55 100644 --- a/common/values/parsed_repeated_field_value_test.cc +++ b/common/values/parsed_repeated_field_value_test.cc @@ -438,10 +438,10 @@ TEST_F(ParsedRepeatedFieldValueTest, Contains) { EXPECT_THAT(value.Contains(DoubleValue(1.0), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); - EXPECT_THAT(value.Contains(StringValue("bar"), descriptor_pool(), + EXPECT_THAT(value.Contains(StringValue::WrapUnsafe("bar"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); - EXPECT_THAT(value.Contains(StringValue("foo"), descriptor_pool(), + EXPECT_THAT(value.Contains(StringValue::WrapUnsafe("foo"), descriptor_pool(), message_factory(), arena()), IsOkAndHolds(BoolValueIs(false))); EXPECT_THAT( diff --git a/common/values/string_value.cc b/common/values/string_value.cc index 9ebde8795..7659e9a71 100644 --- a/common/values/string_value.cc +++ b/common/values/string_value.cc @@ -35,7 +35,6 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "common/internal/byte_string.h" -#include "common/internal/reference_count.h" #include "common/value.h" #include "internal/status_macros.h" #include "internal/strings.h" @@ -618,8 +617,7 @@ absl::StatusOr SubstringImpl(absl::string_view string, uint64_t start) { ".substring(): is greater than .size()"); } -absl::StatusOr SubstringImpl(const absl::Cord& cord, - uint64_t start) { +absl::StatusOr SubstringImpl(const absl::Cord& cord, uint64_t start) { absl::Cord::CharIterator char_begin = cord.char_begin(); absl::Cord::CharIterator char_end = cord.char_end(); size_t size_code_points = 0; @@ -629,14 +627,14 @@ absl::StatusOr SubstringImpl(const absl::Cord& cord, size_t code_units; std::tie(code_point, code_units) = cel::internal::Utf8Decode(char_begin); if (size_code_points == start) { - return cord.Subcord(size_code_units, std::numeric_limits::max()); + return size_code_units; } absl::Cord::Advance(&char_begin, code_units); ++size_code_points; size_code_units += code_units; } if (size_code_points == start) { - return cord; + return size_code_units; } return absl::InvalidArgumentError( ".substring(): is greater than .size()"); @@ -685,17 +683,18 @@ Value StringValue::Substring(int64_t start) const { value_.rep_.medium.size - *status_or_index; result.value_.rep_.medium.data = value_.rep_.medium.data + *status_or_index; - result.value_.rep_.medium.owner = value_.rep_.medium.owner; - common_internal::StrongRef(result.value_.GetMediumReferenceCount()); + result.value_.rep_.medium.arena = value_.rep_.medium.arena; return result; } case common_internal::ByteStringKind::kLarge: { - absl::StatusOr status_or_cord = + absl::StatusOr status_or_index = (SubstringImpl)(value_.GetLarge(), start); - if (!status_or_cord.ok()) { - return ErrorValue(std::move(status_or_cord).status()); + if (!status_or_index.ok()) { + return ErrorValue(std::move(status_or_index).status()); } - return StringValue::Wrap(*std::move(status_or_cord)); + return StringValue(common_internal::ByteString::Wrap( + value_.rep_.large.data, value_.rep_.large.offset + *status_or_index, + value_.rep_.large.size - *status_or_index, value_.rep_.large.arena)); } } } @@ -729,8 +728,9 @@ absl::StatusOr> SubstringImpl( ".size()"); } -absl::StatusOr SubstringImpl(const absl::Cord& cord, uint64_t start, - uint64_t end) { +absl::StatusOr> SubstringImpl(const absl::Cord& cord, + uint64_t start, + uint64_t end) { absl::Cord::CharIterator char_begin = cord.char_begin(); absl::Cord::CharIterator char_end = cord.char_end(); size_t size_code_points = 0; @@ -741,7 +741,7 @@ absl::StatusOr SubstringImpl(const absl::Cord& cord, uint64_t start, start_code_units = size_code_units; } if (size_code_points == end) { - return cord.Subcord(start_code_units, size_code_units - start_code_units); + return std::pair{start_code_units, size_code_units}; } char32_t code_point; size_t code_units; @@ -751,7 +751,7 @@ absl::StatusOr SubstringImpl(const absl::Cord& cord, uint64_t start, size_code_units += code_units; } if (size_code_points == start && start == end) { - return absl::Cord(); + return std::pair{size_code_units, size_code_units}; } return absl::InvalidArgumentError( ".substring(, ): or is greater than " @@ -804,17 +804,20 @@ Value StringValue::Substring(int64_t start, int64_t end) const { (status_or_indices->second - status_or_indices->first); result.value_.rep_.medium.data = value_.rep_.medium.data + status_or_indices->first; - result.value_.rep_.medium.owner = value_.rep_.medium.owner; - common_internal::StrongRef(result.value_.GetMediumReferenceCount()); + result.value_.rep_.medium.arena = value_.rep_.medium.arena; return result; } case common_internal::ByteStringKind::kLarge: { - absl::StatusOr status_or_cord = + absl::StatusOr> status_or_indices = (SubstringImpl)(value_.GetLarge(), start, end); - if (!status_or_cord.ok()) { - return ErrorValue(std::move(status_or_cord).status()); + if (!status_or_indices.ok()) { + return ErrorValue(std::move(status_or_indices).status()); } - return StringValue::Wrap(*std::move(status_or_cord)); + return StringValue(common_internal::ByteString::Wrap( + value_.rep_.large.data, + value_.rep_.large.offset + status_or_indices->first, + status_or_indices->second - status_or_indices->first, + value_.rep_.large.arena)); } } } @@ -841,9 +844,9 @@ bool LowerAsciiImpl(absl::string_view in, std::string* absl_nonnull out) { return true; } -absl::Cord LowerAsciiImpl(const absl::Cord& in) { +bool LowerAsciiImpl(const absl::Cord& in, absl::Cord* absl_nonnull out) { if (in.empty()) { - return in; + return false; } size_t pos = 0; bool needs_conversion = false; @@ -855,9 +858,9 @@ absl::Cord LowerAsciiImpl(const absl::Cord& in) { pos++; } if (!needs_conversion) { - return in; + return false; } - absl::Cord out = in.Subcord(0, pos); + absl::Cord prefix = in.Subcord(0, pos); absl::Cord rest = in.Subcord(pos, in.size() - pos); std::string suffix; suffix.resize(rest.size()); @@ -865,8 +868,9 @@ absl::Cord LowerAsciiImpl(const absl::Cord& in) { for (char c : rest.Chars()) { suffix[current++] = absl::ascii_tolower(c); } - out.Append(std::move(suffix)); - return out; + prefix.Append(std::move(suffix)); + *out = std::move(prefix); + return true; } } // namespace @@ -889,8 +893,13 @@ StringValue StringValue::LowerAscii(google::protobuf::Arena* absl_nonnull arena) } return StringValue::From(std::move(out), arena); } - case common_internal::ByteStringKind::kLarge: - return StringValue::Wrap((LowerAsciiImpl)(value_.GetLarge())); + case common_internal::ByteStringKind::kLarge: { + absl::Cord out; + if (!(LowerAsciiImpl)(value_.GetLarge(), &out)) { + return *this; + } + return StringValue::From(std::move(out), arena); + } } } @@ -916,9 +925,9 @@ bool UpperAsciiImpl(absl::string_view in, std::string* absl_nonnull out) { return true; } -absl::Cord UpperAsciiImpl(const absl::Cord& in) { +bool UpperAsciiImpl(const absl::Cord& in, absl::Cord* absl_nonnull out) { if (in.empty()) { - return in; + return false; } size_t pos = 0; bool needs_conversion = false; @@ -930,9 +939,9 @@ absl::Cord UpperAsciiImpl(const absl::Cord& in) { pos++; } if (!needs_conversion) { - return in; + return false; } - absl::Cord out = in.Subcord(0, pos); + absl::Cord prefix = in.Subcord(0, pos); absl::Cord rest = in.Subcord(pos, in.size() - pos); std::string suffix; suffix.resize(rest.size()); @@ -940,15 +949,15 @@ absl::Cord UpperAsciiImpl(const absl::Cord& in) { for (char c : rest.Chars()) { suffix[current++] = absl::ascii_toupper(c); } - out.Append(std::move(suffix)); - return out; + prefix.Append(std::move(suffix)); + *out = std::move(prefix); + return true; } } // namespace StringValue StringValue::UpperAscii(google::protobuf::Arena* absl_nonnull arena) const { ABSL_DCHECK(arena != nullptr); - switch (value_.GetKind()) { case common_internal::ByteStringKind::kSmall: { std::string out; @@ -964,8 +973,13 @@ StringValue StringValue::UpperAscii(google::protobuf::Arena* absl_nonnull arena) } return StringValue::From(std::move(out), arena); } - case common_internal::ByteStringKind::kLarge: - return StringValue::Wrap((UpperAsciiImpl)(value_.GetLarge())); + case common_internal::ByteStringKind::kLarge: { + absl::Cord out; + if (!(UpperAsciiImpl)(value_.GetLarge(), &out)) { + return *this; + } + return StringValue::From(std::move(out), arena); + } } } @@ -1016,7 +1030,7 @@ std::pair TrimImpl(absl::string_view string) { return {left_trim_bytes, string.size() - last_non_ws_end_bytes}; } -absl::Cord TrimImpl(const absl::Cord& cord) { +std::pair TrimImpl(const absl::Cord& cord) { size_t left_trim_bytes = 0; { absl::Cord::CharIterator begin = cord.char_begin(); @@ -1034,7 +1048,7 @@ absl::Cord TrimImpl(const absl::Cord& cord) { } if (left_trim_bytes == cord.size()) { - return absl::Cord(); + return {left_trim_bytes, 0}; } absl::Cord ltrimmed = @@ -1056,40 +1070,29 @@ absl::Cord TrimImpl(const absl::Cord& cord) { current_pos_bytes += char_len; } } - return ltrimmed.Subcord(0, last_non_ws_end_bytes); + return {left_trim_bytes, ltrimmed.size() - last_non_ws_end_bytes}; } } // namespace StringValue StringValue::Trim() const { + std::pair trims; + size_t size; switch (value_.GetKind()) { - case common_internal::ByteStringKind::kSmall: { - std::pair trims = (TrimImpl)(value_.GetSmall()); - StringValue result; - result.value_.rep_.header.kind = common_internal::ByteStringKind::kSmall; - result.value_.rep_.small.size = - value_.rep_.small.size - trims.first - trims.second; - std::memcpy(result.value_.rep_.small.data, - value_.rep_.small.data + trims.first, - result.value_.rep_.small.size); - result.value_.rep_.small.arena = value_.GetSmallArena(); - return result; - } - case common_internal::ByteStringKind::kMedium: { - std::pair trims = (TrimImpl)(value_.GetMedium()); - StringValue result; - result.value_.rep_.header.kind = common_internal::ByteStringKind::kMedium; - result.value_.rep_.medium.size = - value_.rep_.medium.size - trims.first - trims.second; - result.value_.rep_.medium.data = value_.rep_.medium.data + trims.first; - result.value_.rep_.medium.owner = value_.rep_.medium.owner; - common_internal::StrongRef(result.value_.GetMediumReferenceCount()); - return result; - } - case common_internal::ByteStringKind::kLarge: { - return StringValue::Wrap((TrimImpl)(value_.GetLarge())); - } + case common_internal::ByteStringKind::kSmall: + trims = (TrimImpl)(value_.GetSmall()); + size = value_.GetSmall().size(); + break; + case common_internal::ByteStringKind::kMedium: + trims = (TrimImpl)(value_.GetMedium()); + size = value_.GetMedium().size(); + break; + case common_internal::ByteStringKind::kLarge: + trims = (TrimImpl)(value_.GetLarge()); + size = value_.rep_.large.size; + break; } + return StringValue(value_.Substring(trims.first, size - trims.second)); } namespace { diff --git a/common/values/string_value.h b/common/values/string_value.h index 8045e4b3f..07dadd856 100644 --- a/common/values/string_value.h +++ b/common/values/string_value.h @@ -27,16 +27,12 @@ #include "absl/base/attributes.h" #include "absl/base/nullability.h" -#include "absl/log/absl_check.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/cord.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" -#include "common/allocator.h" -#include "common/arena.h" #include "common/internal/byte_string.h" -#include "common/memory.h" #include "common/type.h" #include "common/value_kind.h" #include "common/values/values.h" @@ -63,75 +59,61 @@ class StringValue final : private common_internal::ValueMixin { static StringValue From(const char* absl_nullable value, google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND); + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return StringValue(common_internal::ByteString::From(value, arena)); + } static StringValue From(absl::string_view value, google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND); - static StringValue From(const absl::Cord& value); + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return StringValue(common_internal::ByteString::From(value, arena)); + } + static StringValue From(const absl::Cord& value, + google::protobuf::Arena* absl_nonnull arena + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return StringValue(common_internal::ByteString::From(value, arena)); + } static StringValue From(std::string&& value, google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND); + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return StringValue( + common_internal::ByteString::From(std::move(value), arena)); + } static StringValue Wrap(absl::string_view value, google::protobuf::Arena* absl_nullable arena - ABSL_ATTRIBUTE_LIFETIME_BOUND); - static StringValue Wrap(absl::string_view value) = delete; - static StringValue Wrap(const absl::Cord& value); - static StringValue Wrap(std::string&& value) = delete; - static StringValue Wrap(std::string&& value, - google::protobuf::Arena* absl_nullable arena - ABSL_ATTRIBUTE_LIFETIME_BOUND) = delete; + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return StringValue(common_internal::ByteString::Wrap(value, arena)); + } + static StringValue Wrap( + const absl::Cord* absl_nonnull value ABSL_ATTRIBUTE_LIFETIME_BOUND, + google::protobuf::Arena* absl_nullable arena ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return StringValue(common_internal::ByteString::Wrap(value, arena)); + } + static StringValue Wrap(std::nullptr_t, google::protobuf::Arena*) = delete; + static StringValue Wrap(std::string&& value, google::protobuf::Arena*) = delete; // Returns a StringValue that aliases the provided string. Caller must ensure // the provided string outlives the use of the returned StringValue. - static StringValue WrapUnsafe(absl::string_view value); + static StringValue WrapUnsafe(absl::string_view value) { + return StringValue(common_internal::ByteString::WrapUnsafe(value)); + } + static StringValue WrapUnsafe(const absl::Cord* absl_nonnull value) { + return StringValue(common_internal::ByteString::WrapUnsafe(value)); + } + static StringValue WrapUnsafe(std::nullptr_t) = delete; static StringValue Concat(const StringValue& lhs, const StringValue& rhs, google::protobuf::Arena* absl_nonnull arena ABSL_ATTRIBUTE_LIFETIME_BOUND); - ABSL_DEPRECATED("Use From") - explicit StringValue(const char* absl_nullable value) : value_(value) {} - - ABSL_DEPRECATED("Use From") - explicit StringValue(absl::string_view value) : value_(value) {} - - ABSL_DEPRECATED("Use From") - explicit StringValue(const absl::Cord& value) : value_(value) {} - - ABSL_DEPRECATED("Use From") - explicit StringValue(std::string&& value) : value_(std::move(value)) {} - - ABSL_DEPRECATED("Use From") - StringValue(Allocator<> allocator, const char* absl_nullable value) - : value_(allocator, value) {} - - ABSL_DEPRECATED("Use From") - StringValue(Allocator<> allocator, absl::string_view value) - : value_(allocator, value) {} - - ABSL_DEPRECATED("Use From") - StringValue(Allocator<> allocator, const absl::Cord& value) - : value_(allocator, value) {} - - ABSL_DEPRECATED("Use From") - StringValue(Allocator<> allocator, std::string&& value) - : value_(allocator, std::move(value)) {} - - ABSL_DEPRECATED("Use Wrap") - StringValue(Borrower borrower, absl::string_view value) - : value_(borrower, value) {} - - ABSL_DEPRECATED("Use Wrap") - StringValue(Borrower borrower, const absl::Cord& value) - : value_(borrower, value) {} - StringValue() = default; StringValue(const StringValue&) = default; StringValue(StringValue&&) = default; StringValue& operator=(const StringValue&) = default; StringValue& operator=(StringValue&&) = default; + explicit StringValue(const BytesValue& other); + constexpr ValueKind kind() const { return kKind; } absl::string_view GetTypeName() const { return StringType::kName; } @@ -186,9 +168,9 @@ class StringValue final : private common_internal::ValueMixin { return value_.Visit(std::forward(visitor)); } - void swap(StringValue& other) noexcept { + friend void swap(StringValue& lhs, StringValue& rhs) noexcept { using std::swap; - swap(value_, other.value_); + swap(lhs.value_, rhs.value_); } size_t Size() const; @@ -354,10 +336,10 @@ class StringValue final : private common_internal::ValueMixin { } private: + friend class BytesValue; friend class common_internal::ValueMixin; friend absl::string_view common_internal::LegacyStringValue( const StringValue& value, bool stable, google::protobuf::Arena* absl_nonnull arena); - friend struct ArenaTraits; explicit StringValue(common_internal::ByteString value) noexcept : value_(std::move(value)) {} @@ -365,8 +347,6 @@ class StringValue final : private common_internal::ValueMixin { common_internal::ByteString value_; }; -inline void swap(StringValue& lhs, StringValue& rhs) noexcept { lhs.swap(rhs); } - inline bool operator==(const StringValue& lhs, absl::string_view rhs) { return lhs.Equals(rhs); } @@ -423,48 +403,6 @@ inline std::ostream& operator<<(std::ostream& out, const StringValue& value) { return out << value.DebugString(); } -inline StringValue StringValue::From(const char* absl_nullable value, - google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND) { - return From(absl::NullSafeStringView(value), arena); -} - -inline StringValue StringValue::From(absl::string_view value, - google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND) { - ABSL_DCHECK(arena != nullptr); - - return StringValue(arena, value); -} - -inline StringValue StringValue::From(const absl::Cord& value) { - return StringValue(value); -} - -inline StringValue StringValue::From(std::string&& value, - google::protobuf::Arena* absl_nonnull arena - ABSL_ATTRIBUTE_LIFETIME_BOUND) { - ABSL_DCHECK(arena != nullptr); - - return StringValue(arena, std::move(value)); -} - -inline StringValue StringValue::Wrap(absl::string_view value, - google::protobuf::Arena* absl_nullable arena - ABSL_ATTRIBUTE_LIFETIME_BOUND) { - ABSL_DCHECK(arena != nullptr); - - return StringValue(Borrower::Arena(arena), value); -} - -inline StringValue StringValue::WrapUnsafe(absl::string_view value) { - return StringValue(common_internal::ByteString::FromExternal(value)); -} - -inline StringValue StringValue::Wrap(const absl::Cord& value) { - return StringValue(value); -} - namespace common_internal { inline absl::string_view LegacyStringValue(const StringValue& value, @@ -475,15 +413,6 @@ inline absl::string_view LegacyStringValue(const StringValue& value, } // namespace common_internal -template <> -struct ArenaTraits { - using constructible = std::true_type; - - static bool trivially_destructible(const StringValue& value) { - return ArenaTraits<>::trivially_destructible(value.value_); - } -}; - } // namespace cel #endif // THIRD_PARTY_CEL_CPP_COMMON_VALUES_STRING_VALUE_H_ diff --git a/common/values/string_value_test.cc b/common/values/string_value_test.cc index b4fa404ae..f17abbf88 100644 --- a/common/values/string_value_test.cc +++ b/common/values/string_value_test.cc @@ -40,185 +40,212 @@ using ::testing::Optional; using StringValueTest = common_internal::ValueTest<>; TEST_F(StringValueTest, Kind) { - EXPECT_EQ(StringValue("foo").kind(), StringValue::kKind); - EXPECT_EQ(Value(StringValue(absl::Cord("foo"))).kind(), StringValue::kKind); + EXPECT_EQ(StringValue::WrapUnsafe("foo").kind(), StringValue::kKind); + EXPECT_EQ(Value(StringValue::From(absl::Cord("foo"), arena())).kind(), + StringValue::kKind); } TEST_F(StringValueTest, DebugString) { { std::ostringstream out; - out << StringValue("foo"); + out << StringValue::WrapUnsafe("foo"); EXPECT_EQ(out.str(), "\"foo\""); } { std::ostringstream out; - out << StringValue(absl::MakeFragmentedCord({"f", "o", "o"})); + out << StringValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), + arena()); EXPECT_EQ(out.str(), "\"foo\""); } { std::ostringstream out; - out << Value(StringValue(absl::Cord("foo"))); + out << Value(StringValue::From(absl::Cord("foo"), arena())); EXPECT_EQ(out.str(), "\"foo\""); } } TEST_F(StringValueTest, ConvertToJson) { auto* message = NewArenaValueMessage(); - EXPECT_THAT(StringValue("foo").ConvertToJson(descriptor_pool(), - message_factory(), message), + EXPECT_THAT(StringValue::WrapUnsafe("foo").ConvertToJson( + descriptor_pool(), message_factory(), message), IsOk()); EXPECT_THAT(*message, EqualsValueTextProto(R"pb(string_value: "foo")pb")); } TEST_F(StringValueTest, NativeValue) { std::string scratch; - EXPECT_EQ(StringValue("foo").NativeString(), "foo"); - EXPECT_EQ(StringValue("foo").NativeString(scratch), "foo"); - EXPECT_EQ(StringValue("foo").NativeCord(), "foo"); + EXPECT_EQ(StringValue::WrapUnsafe("foo").NativeString(), "foo"); + EXPECT_EQ(StringValue::WrapUnsafe("foo").NativeString(scratch), "foo"); + EXPECT_EQ(StringValue::WrapUnsafe("foo").NativeCord(), "foo"); } TEST_F(StringValueTest, TryFlat) { - EXPECT_THAT(StringValue("foo").TryFlat(), Optional(Eq("foo"))); + EXPECT_THAT(StringValue::WrapUnsafe("foo").TryFlat(), Optional(Eq("foo"))); EXPECT_THAT( - StringValue(absl::MakeFragmentedCord({"Hello, World!", "World, Hello!"})) + StringValue::From( + absl::MakeFragmentedCord({"Hello, World!", "World, Hello!"}), arena()) .TryFlat(), Eq(std::nullopt)); } TEST_F(StringValueTest, ToString) { - EXPECT_EQ(StringValue("foo").ToString(), "foo"); - EXPECT_EQ(StringValue(absl::MakeFragmentedCord({"f", "o", "o"})).ToString(), - "foo"); + EXPECT_EQ(StringValue::WrapUnsafe("foo").ToString(), "foo"); + EXPECT_EQ( + StringValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .ToString(), + "foo"); } TEST_F(StringValueTest, CopyToString) { std::string out; - StringValue("foo").CopyToString(&out); + StringValue::WrapUnsafe("foo").CopyToString(&out); EXPECT_EQ(out, "foo"); - StringValue(absl::MakeFragmentedCord({"f", "o", "o"})).CopyToString(&out); + StringValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .CopyToString(&out); EXPECT_EQ(out, "foo"); } TEST_F(StringValueTest, AppendToString) { std::string out; - StringValue("foo").AppendToString(&out); + StringValue::WrapUnsafe("foo").AppendToString(&out); EXPECT_EQ(out, "foo"); - StringValue(absl::MakeFragmentedCord({"f", "o", "o"})).AppendToString(&out); + StringValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .AppendToString(&out); EXPECT_EQ(out, "foofoo"); } TEST_F(StringValueTest, ToCord) { - EXPECT_EQ(StringValue("foo").ToCord(), "foo"); - EXPECT_EQ(StringValue(absl::MakeFragmentedCord({"f", "o", "o"})).ToCord(), - "foo"); + EXPECT_EQ(StringValue::WrapUnsafe("foo").ToCord(), "foo"); + EXPECT_EQ( + StringValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .ToCord(), + "foo"); } TEST_F(StringValueTest, CopyToCord) { absl::Cord out; - StringValue("foo").CopyToCord(&out); + StringValue::WrapUnsafe("foo").CopyToCord(&out); EXPECT_EQ(out, "foo"); - StringValue(absl::MakeFragmentedCord({"f", "o", "o"})).CopyToCord(&out); + StringValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .CopyToCord(&out); EXPECT_EQ(out, "foo"); } TEST_F(StringValueTest, AppendToCord) { absl::Cord out; - StringValue("foo").AppendToCord(&out); + StringValue::WrapUnsafe("foo").AppendToCord(&out); EXPECT_EQ(out, "foo"); - StringValue(absl::MakeFragmentedCord({"f", "o", "o"})).AppendToCord(&out); + StringValue::From(absl::MakeFragmentedCord({"f", "o", "o"}), arena()) + .AppendToCord(&out); EXPECT_EQ(out, "foofoo"); } TEST_F(StringValueTest, NativeTypeId) { - EXPECT_EQ(NativeTypeId::Of(StringValue("foo")), - NativeTypeId::For()); - EXPECT_EQ(NativeTypeId::Of(Value(StringValue(absl::Cord("foo")))), + EXPECT_EQ(NativeTypeId::Of(StringValue::WrapUnsafe("foo")), NativeTypeId::For()); + EXPECT_EQ( + NativeTypeId::Of(Value(StringValue::From(absl::Cord("foo"), arena()))), + NativeTypeId::For()); } TEST_F(StringValueTest, HashValue) { - EXPECT_EQ(absl::HashOf(StringValue("foo")), + EXPECT_EQ(absl::HashOf(StringValue::WrapUnsafe("foo")), absl::HashOf(absl::string_view("foo"))); - EXPECT_EQ(absl::HashOf(StringValue(absl::string_view("foo"))), + EXPECT_EQ(absl::HashOf(StringValue::WrapUnsafe(absl::string_view("foo"))), absl::HashOf(absl::string_view("foo"))); - EXPECT_EQ(absl::HashOf(StringValue(absl::Cord("foo"))), + EXPECT_EQ(absl::HashOf(StringValue::From(absl::Cord("foo"), arena())), absl::HashOf(absl::string_view("foo"))); } TEST_F(StringValueTest, Equality) { - EXPECT_NE(StringValue("foo"), "bar"); - EXPECT_NE("bar", StringValue("foo")); - EXPECT_NE(StringValue("foo"), StringValue("bar")); - EXPECT_NE(StringValue("foo"), absl::Cord("bar")); - EXPECT_NE(absl::Cord("bar"), StringValue("foo")); + EXPECT_NE(StringValue::WrapUnsafe("foo"), "bar"); + EXPECT_NE("bar", StringValue::WrapUnsafe("foo")); + EXPECT_NE(StringValue::WrapUnsafe("foo"), StringValue::WrapUnsafe("bar")); + EXPECT_NE(StringValue::WrapUnsafe("foo"), absl::Cord("bar")); + EXPECT_NE(absl::Cord("bar"), StringValue::WrapUnsafe("foo")); } TEST_F(StringValueTest, LessThan) { - EXPECT_LT(StringValue("bar"), "foo"); - EXPECT_LT("bar", StringValue("foo")); - EXPECT_LT(StringValue("bar"), StringValue("foo")); - EXPECT_LT(StringValue("bar"), absl::Cord("foo")); - EXPECT_LT(absl::Cord("bar"), StringValue("foo")); + EXPECT_LT(StringValue::WrapUnsafe("bar"), "foo"); + EXPECT_LT("bar", StringValue::WrapUnsafe("foo")); + EXPECT_LT(StringValue::WrapUnsafe("bar"), StringValue::WrapUnsafe("foo")); + EXPECT_LT(StringValue::WrapUnsafe("bar"), absl::Cord("foo")); + EXPECT_LT(absl::Cord("bar"), StringValue::WrapUnsafe("foo")); } TEST_F(StringValueTest, StartsWith) { EXPECT_TRUE( - StringValue("This string is large enough to not be stored inline!") - .StartsWith(StringValue("This string is large enough"))); - EXPECT_TRUE( - StringValue("This string is large enough to not be stored inline!") - .StartsWith(StringValue(absl::Cord("This string is large enough")))); + StringValue::WrapUnsafe( + "This string is large enough to not be stored inline!") + .StartsWith(StringValue::WrapUnsafe("This string is large enough"))); + EXPECT_TRUE(StringValue::WrapUnsafe( + "This string is large enough to not be stored inline!") + .StartsWith(StringValue::From( + absl::Cord("This string is large enough"), arena()))); EXPECT_TRUE( - StringValue( - absl::Cord("This string is large enough to not be stored inline!")) - .StartsWith(StringValue("This string is large enough"))); + StringValue::From( + absl::Cord("This string is large enough to not be stored inline!"), + arena()) + .StartsWith(StringValue::WrapUnsafe("This string is large enough"))); EXPECT_TRUE( - StringValue( - absl::Cord("This string is large enough to not be stored inline!")) - .StartsWith(StringValue(absl::Cord("This string is large enough")))); + StringValue::From( + absl::Cord("This string is large enough to not be stored inline!"), + arena()) + .StartsWith(StringValue::From( + absl::Cord("This string is large enough"), arena()))); } TEST_F(StringValueTest, EndsWith) { EXPECT_TRUE( - StringValue("This string is large enough to not be stored inline!") - .EndsWith(StringValue("to not be stored inline!"))); + StringValue::WrapUnsafe( + "This string is large enough to not be stored inline!") + .EndsWith(StringValue::WrapUnsafe("to not be stored inline!"))); + EXPECT_TRUE(StringValue::WrapUnsafe( + "This string is large enough to not be stored inline!") + .EndsWith(StringValue::From( + absl::Cord("to not be stored inline!"), arena()))); EXPECT_TRUE( - StringValue("This string is large enough to not be stored inline!") - .EndsWith(StringValue(absl::Cord("to not be stored inline!")))); + StringValue::From( + absl::Cord("This string is large enough to not be stored inline!"), + arena()) + .EndsWith(StringValue::WrapUnsafe("to not be stored inline!"))); EXPECT_TRUE( - StringValue( - absl::Cord("This string is large enough to not be stored inline!")) - .EndsWith(StringValue("to not be stored inline!"))); - EXPECT_TRUE( - StringValue( - absl::Cord("This string is large enough to not be stored inline!")) - .EndsWith(StringValue(absl::Cord("to not be stored inline!")))); + StringValue::From( + absl::Cord("This string is large enough to not be stored inline!"), + arena()) + .EndsWith(StringValue::From(absl::Cord("to not be stored inline!"), + arena()))); } TEST_F(StringValueTest, Contains) { + EXPECT_TRUE(StringValue::WrapUnsafe( + "This string is large enough to not be stored inline!") + .Contains(StringValue::WrapUnsafe("string is large enough"))); + EXPECT_TRUE(StringValue::WrapUnsafe( + "This string is large enough to not be stored inline!") + .Contains(StringValue::From( + absl::Cord("string is large enough"), arena()))); EXPECT_TRUE( - StringValue("This string is large enough to not be stored inline!") - .Contains(StringValue("string is large enough"))); - EXPECT_TRUE( - StringValue("This string is large enough to not be stored inline!") - .Contains(StringValue(absl::Cord("string is large enough")))); - EXPECT_TRUE( - StringValue( - absl::Cord("This string is large enough to not be stored inline!")) - .Contains(StringValue("string is large enough"))); + StringValue::From( + absl::Cord("This string is large enough to not be stored inline!"), + arena()) + .Contains(StringValue::WrapUnsafe("string is large enough"))); EXPECT_TRUE( - StringValue( - absl::Cord("This string is large enough to not be stored inline!")) - .Contains(StringValue(absl::Cord("string is large enough")))); + StringValue::From( + absl::Cord("This string is large enough to not be stored inline!"), + arena()) + .Contains(StringValue::From(absl::Cord("string is large enough"), + arena()))); } TEST_F(StringValueTest, IndexOf) { - StringValue big_string = - StringValue("This string is large enough to not be stored inline!"); - StringValue big_string_cord = StringValue( - absl::Cord("This string is large enough to not be stored inline!")); - StringValue small_string = StringValue("is"); - StringValue small_string_cord = StringValue(absl::Cord("is")); + StringValue big_string = StringValue::WrapUnsafe( + "This string is large enough to not be stored inline!"); + StringValue big_string_cord = StringValue::From( + absl::Cord("This string is large enough to not be stored inline!"), + arena()); + StringValue small_string = StringValue::WrapUnsafe("is"); + StringValue small_string_cord = StringValue::From(absl::Cord("is"), arena()); EXPECT_THAT(big_string.IndexOf(small_string), Optional(Eq(2))); EXPECT_THAT(big_string.IndexOf(small_string_cord), Optional(Eq(2))); @@ -249,66 +276,81 @@ TEST_F(StringValueTest, IndexOf) { } TEST_F(StringValueTest, LowerAscii) { - EXPECT_EQ(StringValue("UPPER lower").LowerAscii(arena()), "upper lower"); - EXPECT_EQ(StringValue(absl::Cord("UPPER lower")).LowerAscii(arena()), + EXPECT_EQ(StringValue::WrapUnsafe("UPPER lower").LowerAscii(arena()), "upper lower"); - EXPECT_EQ(StringValue("upper lower").LowerAscii(arena()), "upper lower"); - EXPECT_EQ(StringValue(absl::Cord("upper lower")).LowerAscii(arena()), + EXPECT_EQ( + StringValue::From(absl::Cord("UPPER lower"), arena()).LowerAscii(arena()), + "upper lower"); + EXPECT_EQ(StringValue::WrapUnsafe("upper lower").LowerAscii(arena()), "upper lower"); - EXPECT_EQ(StringValue("").LowerAscii(arena()), ""); - EXPECT_EQ(StringValue(absl::Cord("")).LowerAscii(arena()), ""); + EXPECT_EQ( + StringValue::From(absl::Cord("upper lower"), arena()).LowerAscii(arena()), + "upper lower"); + EXPECT_EQ(StringValue::WrapUnsafe("").LowerAscii(arena()), ""); + EXPECT_EQ(StringValue::From(absl::Cord(""), arena()).LowerAscii(arena()), ""); const std::string kLongMixed = "A long STRING with MiXeD case to test conversion to lower case!"; const std::string kLongLower = "a long string with mixed case to test conversion to lower case!"; - EXPECT_EQ(StringValue(absl::Cord(kLongMixed)).LowerAscii(arena()), - kLongLower); + EXPECT_EQ( + StringValue::From(absl::Cord(kLongMixed), arena()).LowerAscii(arena()), + kLongLower); std::string very_long_mixed(10000, 'A'); std::string very_long_lower(10000, 'a'); + EXPECT_EQ(StringValue::From( + absl::MakeFragmentedCord({very_long_mixed.substr(0, 5000), + very_long_mixed.substr(5000)}), + arena()) + .LowerAscii(arena()), + very_long_lower); EXPECT_EQ( - StringValue(absl::MakeFragmentedCord({very_long_mixed.substr(0, 5000), - very_long_mixed.substr(5000)})) + StringValue::From(absl::MakeFragmentedCord({"hello", "WORLD"}), arena()) .LowerAscii(arena()), - very_long_lower); - EXPECT_EQ(StringValue(absl::MakeFragmentedCord({"hello", "WORLD"})) - .LowerAscii(arena()), - "helloworld"); + "helloworld"); } TEST_F(StringValueTest, UpperAscii) { - EXPECT_EQ(StringValue("UPPER lower").UpperAscii(arena()), "UPPER LOWER"); - EXPECT_EQ(StringValue(absl::Cord("UPPER lower")).UpperAscii(arena()), + EXPECT_EQ(StringValue::WrapUnsafe("UPPER lower").UpperAscii(arena()), "UPPER LOWER"); - EXPECT_EQ(StringValue("UPPER LOWER").UpperAscii(arena()), "UPPER LOWER"); - EXPECT_EQ(StringValue(absl::Cord("UPPER LOWER")).UpperAscii(arena()), + EXPECT_EQ( + StringValue::From(absl::Cord("UPPER lower"), arena()).UpperAscii(arena()), + "UPPER LOWER"); + EXPECT_EQ(StringValue::WrapUnsafe("UPPER LOWER").UpperAscii(arena()), "UPPER LOWER"); - EXPECT_EQ(StringValue("").UpperAscii(arena()), ""); - EXPECT_EQ(StringValue(absl::Cord("")).UpperAscii(arena()), ""); + EXPECT_EQ( + StringValue::From(absl::Cord("UPPER LOWER"), arena()).UpperAscii(arena()), + "UPPER LOWER"); + EXPECT_EQ(StringValue::WrapUnsafe("").UpperAscii(arena()), ""); + EXPECT_EQ(StringValue::From(absl::Cord(""), arena()).UpperAscii(arena()), ""); const std::string kLongMixed = "A long STRING with MiXeD case to test conversion to UPPER case!"; const std::string kLongUpper = "A LONG STRING WITH MIXED CASE TO TEST CONVERSION TO UPPER CASE!"; - EXPECT_EQ(StringValue(absl::Cord(kLongMixed)).UpperAscii(arena()), - kLongUpper); + EXPECT_EQ( + StringValue::From(absl::Cord(kLongMixed), arena()).UpperAscii(arena()), + kLongUpper); std::string very_long_mixed(10000, 'a'); std::string very_long_upper(10000, 'A'); + EXPECT_EQ(StringValue::From( + absl::MakeFragmentedCord({very_long_mixed.substr(0, 5000), + very_long_mixed.substr(5000)}), + arena()) + .UpperAscii(arena()), + very_long_upper); EXPECT_EQ( - StringValue(absl::MakeFragmentedCord({very_long_mixed.substr(0, 5000), - very_long_mixed.substr(5000)})) + StringValue::From(absl::MakeFragmentedCord({"HELLO", "world"}), arena()) .UpperAscii(arena()), - very_long_upper); - EXPECT_EQ(StringValue(absl::MakeFragmentedCord({"HELLO", "world"})) - .UpperAscii(arena()), - "HELLOWORLD"); + "HELLOWORLD"); } TEST_F(StringValueTest, LastIndexOf) { - StringValue big_string = - StringValue("This string is large enough to not be stored inline!"); - StringValue big_string_cord = StringValue( - absl::Cord("This string is large enough to not be stored inline!")); - StringValue small_string = StringValue("is"); - StringValue small_string_cord = StringValue(absl::Cord("is")); + StringValue big_string = StringValue::WrapUnsafe( + "This string is large enough to not be stored inline!"); + StringValue big_string_cord = StringValue::From( + absl::Cord("This string is large enough to not be stored inline!"), + arena()); + StringValue small_string = StringValue::WrapUnsafe("is"); + StringValue small_string_cord = StringValue::From(absl::Cord("is"), arena()); EXPECT_THAT(big_string.LastIndexOf(small_string), Optional(Eq(12))); EXPECT_THAT(big_string.LastIndexOf(small_string_cord), Optional(Eq(12))); @@ -346,12 +388,12 @@ TEST_F(StringValueTest, LastIndexOf) { TEST_F(StringValueTest, Trim) { using ::cel::test::StringValueIs; - StringValue unpadded = StringValue("no padding"); - StringValue front_padded = StringValue(" \t\r\nno padding"); - StringValue back_padded = StringValue("no padding \t\r\n"); - StringValue both_padded = StringValue(" \t\r\nno padding \t\r\n"); - StringValue whitespace = StringValue(" \t\r\n"); - StringValue empty = StringValue(""); + StringValue unpadded = StringValue::WrapUnsafe("no padding"); + StringValue front_padded = StringValue::WrapUnsafe(" \t\r\nno padding"); + StringValue back_padded = StringValue::WrapUnsafe("no padding \t\r\n"); + StringValue both_padded = StringValue::WrapUnsafe(" \t\r\nno padding \t\r\n"); + StringValue whitespace = StringValue::WrapUnsafe(" \t\r\n"); + StringValue empty = StringValue::WrapUnsafe(""); EXPECT_THAT(unpadded.Trim(), StringValueIs("no padding")); EXPECT_THAT(front_padded.Trim(), StringValueIs("no padding")); @@ -360,13 +402,17 @@ TEST_F(StringValueTest, Trim) { EXPECT_THAT(whitespace.Trim(), StringValueIs("")); EXPECT_THAT(empty.Trim(), StringValueIs("")); - StringValue unpadded_cord = StringValue(absl::Cord("no padding")); - StringValue front_padded_cord = StringValue(absl::Cord(" \t\r\nno padding")); - StringValue back_padded_cord = StringValue(absl::Cord("no padding \t\r\n")); + StringValue unpadded_cord = + StringValue::From(absl::Cord("no padding"), arena()); + StringValue front_padded_cord = + StringValue::From(absl::Cord(" \t\r\nno padding"), arena()); + StringValue back_padded_cord = + StringValue::From(absl::Cord("no padding \t\r\n"), arena()); StringValue both_padded_cord = - StringValue(absl::Cord(" \t\r\nno padding \t\r\n")); - StringValue whitespace_cord = StringValue(absl::Cord(" \t\r\n")); - StringValue empty_cord = StringValue(absl::Cord("")); + StringValue::From(absl::Cord(" \t\r\nno padding \t\r\n"), arena()); + StringValue whitespace_cord = + StringValue::From(absl::Cord(" \t\r\n"), arena()); + StringValue empty_cord = StringValue::From(absl::Cord(""), arena()); EXPECT_THAT(unpadded_cord.Trim(), StringValueIs("no padding")); EXPECT_THAT(front_padded_cord.Trim(), StringValueIs("no padding")); @@ -379,14 +425,16 @@ TEST_F(StringValueTest, Trim) { TEST_F(StringValueTest, CharAt) { using ::cel::test::ErrorValueIs; using ::cel::test::StringValueIs; - StringValue big_string = - StringValue("This string is large enough to not be stored inline!"); - StringValue big_string_cord = StringValue( - absl::Cord("This string is large enough to not be stored inline!")); - StringValue small_string = StringValue("abc"); - StringValue small_string_cord = StringValue(absl::Cord("abc")); - StringValue unicode_string = StringValue("aμc"); - StringValue unicode_string_cord = StringValue(absl::Cord("aμc")); + StringValue big_string = StringValue::WrapUnsafe( + "This string is large enough to not be stored inline!"); + StringValue big_string_cord = StringValue::From( + absl::Cord("This string is large enough to not be stored inline!"), + arena()); + StringValue small_string = StringValue::WrapUnsafe("abc"); + StringValue small_string_cord = StringValue::From(absl::Cord("abc"), arena()); + StringValue unicode_string = StringValue::WrapUnsafe("aμc"); + StringValue unicode_string_cord = + StringValue::From(absl::Cord("aμc"), arena()); EXPECT_THAT(big_string.CharAt(0), StringValueIs("T")); EXPECT_THAT(big_string_cord.CharAt(0), StringValueIs("T")); @@ -419,8 +467,8 @@ TEST_F(StringValueTest, Substring) { // as a large (cord-backed) value. The substring length must be measured in // code units, not code points, otherwise the cord overload truncates a // multi-byte character or underflows the length. - StringValue unicode_cord = StringValue(absl::Cord("€€€€€€")); - StringValue unicode_view = StringValue("€€€€€€"); + StringValue unicode_cord = StringValue::From(absl::Cord("€€€€€€"), arena()); + StringValue unicode_view = StringValue::WrapUnsafe("€€€€€€"); EXPECT_THAT(unicode_cord.Substring(0, 2), StringValueIs("€€")); EXPECT_THAT(unicode_view.Substring(0, 2), StringValueIs("€€")); @@ -445,7 +493,7 @@ TEST_F(StringValueTest, Join) { using ::cel::test::ErrorValueIs; using ::cel::test::StringValueIs; - StringValue separator(","); + StringValue separator = StringValue::WrapUnsafe(","); Value result; // Empty list. @@ -458,7 +506,7 @@ TEST_F(StringValueTest, Join) { // Single element list. auto list_builder1 = NewListValueBuilder(arena()); - ASSERT_THAT(list_builder1->Add(StringValue("foo")), IsOk()); + ASSERT_THAT(list_builder1->Add(StringValue::WrapUnsafe("foo")), IsOk()); auto list1 = std::move(*list_builder1).Build(); EXPECT_THAT(separator.Join(list1, descriptor_pool(), message_factory(), arena(), &result), @@ -467,9 +515,9 @@ TEST_F(StringValueTest, Join) { // Multi element list. auto list_builder2 = NewListValueBuilder(arena()); - ASSERT_THAT(list_builder2->Add(StringValue("foo")), IsOk()); - ASSERT_THAT(list_builder2->Add(StringValue("bar")), IsOk()); - ASSERT_THAT(list_builder2->Add(StringValue("baz")), IsOk()); + ASSERT_THAT(list_builder2->Add(StringValue::WrapUnsafe("foo")), IsOk()); + ASSERT_THAT(list_builder2->Add(StringValue::WrapUnsafe("bar")), IsOk()); + ASSERT_THAT(list_builder2->Add(StringValue::WrapUnsafe("baz")), IsOk()); auto list2 = std::move(*list_builder2).Build(); EXPECT_THAT(separator.Join(list2, descriptor_pool(), message_factory(), arena(), &result), @@ -487,7 +535,7 @@ TEST_F(StringValueTest, Join) { // List with string and non-string. auto list_builder4 = NewListValueBuilder(arena()); - ASSERT_THAT(list_builder4->Add(StringValue("foo")), IsOk()); + ASSERT_THAT(list_builder4->Add(StringValue::WrapUnsafe("foo")), IsOk()); ASSERT_THAT(list_builder4->Add(IntValue(1)), IsOk()); auto list4 = std::move(*list_builder4).Build(); EXPECT_THAT(separator.Join(list4, descriptor_pool(), message_factory(), @@ -500,20 +548,24 @@ TEST_F(StringValueTest, Reverse) { using ::cel::test::StringValueIs; EXPECT_THAT(StringValue().Reverse(arena()), StringValueIs("")); - EXPECT_THAT(StringValue("").Reverse(arena()), StringValueIs("")); - EXPECT_THAT(StringValue("hello").Reverse(arena()), StringValueIs("olleh")); - EXPECT_THAT(StringValue("aμc").Reverse(arena()), StringValueIs("cμa")); + EXPECT_THAT(StringValue::WrapUnsafe("").Reverse(arena()), StringValueIs("")); + EXPECT_THAT(StringValue::WrapUnsafe("hello").Reverse(arena()), + StringValueIs("olleh")); + EXPECT_THAT(StringValue::WrapUnsafe("aμc").Reverse(arena()), + StringValueIs("cμa")); EXPECT_THAT( - StringValue("This string is large enough to not be stored inline!") + StringValue::WrapUnsafe( + "This string is large enough to not be stored inline!") .Reverse(arena()), StringValueIs("!enilni derots eb ton ot hguone egral si gnirts sihT")); - EXPECT_THAT(StringValue(absl::Cord("hello")).Reverse(arena()), + EXPECT_THAT(StringValue::From(absl::Cord("hello"), arena()).Reverse(arena()), StringValueIs("olleh")); - EXPECT_THAT(StringValue(absl::Cord("aμc")).Reverse(arena()), + EXPECT_THAT(StringValue::From(absl::Cord("aμc"), arena()).Reverse(arena()), StringValueIs("cμa")); EXPECT_THAT( - StringValue( - absl::Cord("This string is large enough to not be stored inline!")) + StringValue::From( + absl::Cord("This string is large enough to not be stored inline!"), + arena()) .Reverse(arena()), StringValueIs("!enilni derots eb ton ot hguone egral si gnirts sihT")); } diff --git a/common/values/value_variant.cc b/common/values/value_variant.cc index 1c287239c..7c9981d83 100644 --- a/common/values/value_variant.cc +++ b/common/values/value_variant.cc @@ -21,9 +21,7 @@ #include "absl/base/optimization.h" #include "absl/log/absl_check.h" -#include "common/values/bytes_value.h" #include "common/values/error_value.h" -#include "common/values/string_value.h" #include "common/values/unknown_value.h" #include "common/values/values.h" @@ -33,13 +31,6 @@ void ValueVariant::SlowCopyConstruct(const ValueVariant& other) noexcept { ABSL_DCHECK((flags_ & ValueFlags::kNonTrivial) == ValueFlags::kNonTrivial); switch (index_) { - case ValueIndex::kBytes: - ::new (static_cast(&raw_[0])) BytesValue(*other.At()); - break; - case ValueIndex::kString: - ::new (static_cast(&raw_[0])) - StringValue(*other.At()); - break; case ValueIndex::kError: ::new (static_cast(&raw_[0])) ErrorValue(*other.At()); break; @@ -56,14 +47,6 @@ void ValueVariant::SlowMoveConstruct(ValueVariant& other) noexcept { ABSL_DCHECK((flags_ & ValueFlags::kNonTrivial) == ValueFlags::kNonTrivial); switch (index_) { - case ValueIndex::kBytes: - ::new (static_cast(&raw_[0])) - BytesValue(std::move(*other.At())); - break; - case ValueIndex::kString: - ::new (static_cast(&raw_[0])) - StringValue(std::move(*other.At())); - break; case ValueIndex::kError: ::new (static_cast(&raw_[0])) ErrorValue(std::move(*other.At())); @@ -81,12 +64,6 @@ void ValueVariant::SlowDestruct() noexcept { ABSL_DCHECK((flags_ & ValueFlags::kNonTrivial) == ValueFlags::kNonTrivial); switch (index_) { - case ValueIndex::kBytes: - At()->~BytesValue(); - break; - case ValueIndex::kString: - At()->~StringValue(); - break; case ValueIndex::kError: At()->~ErrorValue(); break; @@ -104,14 +81,6 @@ void ValueVariant::SlowCopyAssign(const ValueVariant& other, bool trivial, if (trivial) { switch (other.index_) { - case ValueIndex::kBytes: - ::new (static_cast(&raw_[0])) - BytesValue(*other.At()); - break; - case ValueIndex::kString: - ::new (static_cast(&raw_[0])) - StringValue(*other.At()); - break; case ValueIndex::kError: ::new (static_cast(&raw_[0])) ErrorValue(*other.At()); @@ -128,12 +97,6 @@ void ValueVariant::SlowCopyAssign(const ValueVariant& other, bool trivial, flags_ = other.flags_; } else if (other_trivial) { switch (index_) { - case ValueIndex::kBytes: - At()->~BytesValue(); - break; - case ValueIndex::kString: - At()->~StringValue(); - break; case ValueIndex::kError: At()->~ErrorValue(); break; @@ -146,82 +109,8 @@ void ValueVariant::SlowCopyAssign(const ValueVariant& other, bool trivial, FastCopyAssign(other); } else { switch (index_) { - case ValueIndex::kBytes: - switch (other.index_) { - case ValueIndex::kBytes: - *At() = *other.At(); - break; - case ValueIndex::kString: - At()->~BytesValue(); - ::new (static_cast(&raw_[0])) - StringValue(*other.At()); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kError: - At()->~BytesValue(); - ::new (static_cast(&raw_[0])) - ErrorValue(*other.At()); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kUnknown: - At()->~BytesValue(); - ::new (static_cast(&raw_[0])) - UnknownValue(*other.At()); - index_ = other.index_; - kind_ = other.kind_; - break; - default: - ABSL_UNREACHABLE(); - } - break; - case ValueIndex::kString: - switch (other.index_) { - case ValueIndex::kBytes: - At()->~StringValue(); - ::new (static_cast(&raw_[0])) - BytesValue(*other.At()); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kString: - *At() = *other.At(); - break; - case ValueIndex::kError: - At()->~StringValue(); - ::new (static_cast(&raw_[0])) - ErrorValue(*other.At()); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kUnknown: - At()->~StringValue(); - ::new (static_cast(&raw_[0])) - UnknownValue(*other.At()); - index_ = other.index_; - kind_ = other.kind_; - break; - default: - ABSL_UNREACHABLE(); - } - break; case ValueIndex::kError: switch (other.index_) { - case ValueIndex::kBytes: - At()->~ErrorValue(); - ::new (static_cast(&raw_[0])) - BytesValue(*other.At()); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kString: - At()->~ErrorValue(); - ::new (static_cast(&raw_[0])) - StringValue(*other.At()); - index_ = other.index_; - kind_ = other.kind_; - break; case ValueIndex::kError: *At() = *other.At(); break; @@ -238,20 +127,6 @@ void ValueVariant::SlowCopyAssign(const ValueVariant& other, bool trivial, break; case ValueIndex::kUnknown: switch (other.index_) { - case ValueIndex::kBytes: - At()->~UnknownValue(); - ::new (static_cast(&raw_[0])) - BytesValue(*other.At()); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kString: - At()->~UnknownValue(); - ::new (static_cast(&raw_[0])) - StringValue(*other.At()); - index_ = other.index_; - kind_ = other.kind_; - break; case ValueIndex::kError: At()->~UnknownValue(); ::new (static_cast(&raw_[0])) @@ -283,14 +158,6 @@ void ValueVariant::SlowMoveAssign(ValueVariant& other, bool trivial, if (trivial) { switch (other.index_) { - case ValueIndex::kBytes: - ::new (static_cast(&raw_[0])) - BytesValue(std::move(*other.At())); - break; - case ValueIndex::kString: - ::new (static_cast(&raw_[0])) - StringValue(std::move(*other.At())); - break; case ValueIndex::kError: ::new (static_cast(&raw_[0])) ErrorValue(std::move(*other.At())); @@ -307,12 +174,6 @@ void ValueVariant::SlowMoveAssign(ValueVariant& other, bool trivial, flags_ = other.flags_; } else if (other_trivial) { switch (index_) { - case ValueIndex::kBytes: - At()->~BytesValue(); - break; - case ValueIndex::kString: - At()->~StringValue(); - break; case ValueIndex::kError: At()->~ErrorValue(); break; @@ -325,82 +186,8 @@ void ValueVariant::SlowMoveAssign(ValueVariant& other, bool trivial, FastMoveAssign(other); } else { switch (index_) { - case ValueIndex::kBytes: - switch (other.index_) { - case ValueIndex::kBytes: - *At() = std::move(*other.At()); - break; - case ValueIndex::kString: - At()->~BytesValue(); - ::new (static_cast(&raw_[0])) - StringValue(std::move(*other.At())); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kError: - At()->~BytesValue(); - ::new (static_cast(&raw_[0])) - ErrorValue(std::move(*other.At())); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kUnknown: - At()->~BytesValue(); - ::new (static_cast(&raw_[0])) - UnknownValue(std::move(*other.At())); - index_ = other.index_; - kind_ = other.kind_; - break; - default: - ABSL_UNREACHABLE(); - } - break; - case ValueIndex::kString: - switch (other.index_) { - case ValueIndex::kBytes: - At()->~StringValue(); - ::new (static_cast(&raw_[0])) - BytesValue(std::move(*other.At())); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kString: - *At() = std::move(*other.At()); - break; - case ValueIndex::kError: - At()->~StringValue(); - ::new (static_cast(&raw_[0])) - ErrorValue(std::move(*other.At())); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kUnknown: - At()->~StringValue(); - ::new (static_cast(&raw_[0])) - UnknownValue(std::move(*other.At())); - index_ = other.index_; - kind_ = other.kind_; - break; - default: - ABSL_UNREACHABLE(); - } - break; case ValueIndex::kError: switch (other.index_) { - case ValueIndex::kBytes: - At()->~ErrorValue(); - ::new (static_cast(&raw_[0])) - BytesValue(std::move(*other.At())); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kString: - At()->~ErrorValue(); - ::new (static_cast(&raw_[0])) - StringValue(std::move(*other.At())); - index_ = other.index_; - kind_ = other.kind_; - break; case ValueIndex::kError: *At() = std::move(*other.At()); break; @@ -417,20 +204,6 @@ void ValueVariant::SlowMoveAssign(ValueVariant& other, bool trivial, break; case ValueIndex::kUnknown: switch (other.index_) { - case ValueIndex::kBytes: - At()->~UnknownValue(); - ::new (static_cast(&raw_[0])) - BytesValue(std::move(*other.At())); - index_ = other.index_; - kind_ = other.kind_; - break; - case ValueIndex::kString: - At()->~UnknownValue(); - ::new (static_cast(&raw_[0])) - StringValue(std::move(*other.At())); - index_ = other.index_; - kind_ = other.kind_; - break; case ValueIndex::kError: At()->~UnknownValue(); ::new (static_cast(&raw_[0])) @@ -463,16 +236,6 @@ void ValueVariant::SlowSwap(ValueVariant& lhs, ValueVariant& rhs, // NOLINTNEXTLINE(bugprone-undefined-memory-manipulation) std::memcpy(tmp, std::addressof(lhs), sizeof(ValueVariant)); switch (rhs.index_) { - case ValueIndex::kBytes: - ::new (static_cast(&lhs.raw_[0])) - BytesValue(*rhs.At()); - rhs.At()->~BytesValue(); - break; - case ValueIndex::kString: - ::new (static_cast(&lhs.raw_[0])) - StringValue(*rhs.At()); - rhs.At()->~StringValue(); - break; case ValueIndex::kError: ::new (static_cast(&lhs.raw_[0])) ErrorValue(*rhs.At()); @@ -498,16 +261,6 @@ void ValueVariant::SlowSwap(ValueVariant& lhs, ValueVariant& rhs, // NOLINTNEXTLINE(bugprone-undefined-memory-manipulation) std::memcpy(tmp, std::addressof(rhs), sizeof(ValueVariant)); switch (lhs.index_) { - case ValueIndex::kBytes: - ::new (static_cast(&rhs.raw_[0])) - BytesValue(*lhs.At()); - lhs.At()->~BytesValue(); - break; - case ValueIndex::kString: - ::new (static_cast(&rhs.raw_[0])) - StringValue(*lhs.At()); - lhs.At()->~StringValue(); - break; case ValueIndex::kError: ::new (static_cast(&rhs.raw_[0])) ErrorValue(*lhs.At()); diff --git a/common/values/value_variant.h b/common/values/value_variant.h index b05511e3c..5a26a742f 100644 --- a/common/values/value_variant.h +++ b/common/values/value_variant.h @@ -86,10 +86,9 @@ enum class ValueIndex : uint8_t { kParsedMessage, kCustomStruct, kOpaque, - - // Keep non-trivial alternatives together to aid in compiling optimizations. kBytes, kString, + // Keep non-trivial alternatives together to aid in compiling optimizations. kError, kUnknown, }; @@ -355,9 +354,7 @@ struct ValueAlternative { static constexpr bool kAlwaysTrivial = false; static ValueFlags Flags(const BytesValue* absl_nonnull alternative) { - return ArenaTraits::trivially_destructible(*alternative) - ? ValueFlags::kNone - : ValueFlags::kNonTrivial; + return ValueFlags::kNone; } }; @@ -368,9 +365,7 @@ struct ValueAlternative { static constexpr bool kAlwaysTrivial = false; static ValueFlags Flags(const StringValue* absl_nonnull alternative) { - return ArenaTraits::trivially_destructible(*alternative) - ? ValueFlags::kNone - : ValueFlags::kNonTrivial; + return ValueFlags::kNone; } }; diff --git a/common/values/value_variant_test.cc b/common/values/value_variant_test.cc index 1fd3629aa..968dca0d9 100644 --- a/common/values/value_variant_test.cc +++ b/common/values/value_variant_test.cc @@ -63,16 +63,16 @@ struct DefaultValue { template <> struct DefaultValue { BytesValue operator()() const { - return BytesValue( - absl::Cord("Some somewhat large string that is not storable inline!")); + return BytesValue::WrapUnsafe( + "Some somewhat large string that is not storable inline!"); } }; template <> struct DefaultValue { StringValue operator()() const { - return StringValue( - absl::Cord("Some somewhat large string that is not storable inline!")); + return StringValue::WrapUnsafe( + "Some somewhat large string that is not storable inline!"); } }; diff --git a/conformance/policy/policy_conformance_test.cc b/conformance/policy/policy_conformance_test.cc index 53fbcb419..c325dace8 100644 --- a/conformance/policy/policy_conformance_test.cc +++ b/conformance/policy/policy_conformance_test.cc @@ -136,9 +136,9 @@ cel::Value LocationCode(const cel::StringValue& ip, const google::protobuf::DescriptorPool* pool, google::protobuf::MessageFactory* factory, google::protobuf::Arena* arena) { std::string ip_str = ip.ToString(); - if (ip_str == "10.0.0.1") return cel::StringValue(arena, "us"); - if (ip_str == "10.0.0.2") return cel::StringValue(arena, "de"); - return cel::StringValue(arena, "ir"); + if (ip_str == "10.0.0.1") return cel::StringValue::WrapUnsafe("us"); + if (ip_str == "10.0.0.2") return cel::StringValue::WrapUnsafe("de"); + return cel::StringValue::WrapUnsafe("ir"); } // TODO(uncreated-issue/92): This should be migrated to use the testrunner utility @@ -209,7 +209,9 @@ class InputEvaluator { cel::Activation activation; EvaluateOptions options; options.message_factory = message_factory; - return program->Evaluate(arena, activation, options); + CEL_ASSIGN_OR_RETURN(auto result, + program->Evaluate(arena, activation, options)); + return result.Clone(arena); } private: diff --git a/eval/compiler/flat_expr_builder.cc b/eval/compiler/flat_expr_builder.cc index 53f7cf0c4..5a2490976 100644 --- a/eval/compiler/flat_expr_builder.cc +++ b/eval/compiler/flat_expr_builder.cc @@ -716,7 +716,7 @@ class FlatExprVisitor : public cel::AstVisitor { } absl::StatusOr converted_value = - ConvertConstant(const_expr, cel::NewDeleteAllocator()); + ConvertConstant(const_expr, extension_context_.MutableArena()); if (!converted_value.ok()) { SetProgressStatusIfError(converted_value.status()); @@ -995,7 +995,7 @@ class FlatExprVisitor : public cel::AstVisitor { return; } - StringValue field = cel::StringValue(select_expr.field()); + std::string field = select_expr.field(); std::optional struct_type; std::optional field_type; if (options_.enable_typed_field_access) { diff --git a/eval/eval/comprehension_slots_test.cc b/eval/eval/comprehension_slots_test.cc index 5f869d7cb..3dba2f80e 100644 --- a/eval/eval/comprehension_slots_test.cc +++ b/eval/eval/comprehension_slots_test.cc @@ -38,7 +38,7 @@ TEST(ComprehensionSlots, Basic) { ComprehensionSlots::Slot* slot0 = slots.Get(0); EXPECT_FALSE(slot0->Has()); - slots.Set(0, cel::StringValue("abcd"), + slots.Set(0, cel::StringValue::WrapUnsafe("abcd"), AttributeTrail(Attribute("fake_attr"))); ASSERT_TRUE(slot0->Has()); @@ -55,7 +55,7 @@ TEST(ComprehensionSlots, Basic) { slots.ClearSlot(0); EXPECT_FALSE(slot0->Has()); - slots.Set(3, cel::StringValue("abcd"), + slots.Set(3, cel::StringValue::WrapUnsafe("abcd"), AttributeTrail(Attribute("fake_attr"))); auto* slot3 = slots.Get(3); diff --git a/eval/eval/function_step_test.cc b/eval/eval/function_step_test.cc index 3d3bae34d..1bc6b35ee 100644 --- a/eval/eval/function_step_test.cc +++ b/eval/eval/function_step_test.cc @@ -1186,7 +1186,7 @@ TEST_F(DirectFunctionStepTest, NoOverload) { std::vector> deps; deps.push_back(CreateConstValueDirectStep(cel::IntValue(1))); - deps.push_back(CreateConstValueDirectStep(cel::StringValue("2"))); + deps.push_back(CreateConstValueDirectStep(cel::StringValue::WrapUnsafe("2"))); auto expr = CreateDirectFunctionStep(-1, call, std::move(deps), GetOverloads(cel::builtin::kAdd, 2)); diff --git a/eval/eval/select_step.cc b/eval/eval/select_step.cc index 0adb77fd6..cb80eae34 100644 --- a/eval/eval/select_step.cc +++ b/eval/eval/select_step.cc @@ -206,11 +206,10 @@ absl::Status PerformOptionalGet(const Value& target, absl::string_view field, // message. class SelectStep : public ExpressionStepBase { public: - SelectStep(StringValue value, bool test_field_presence, int64_t expr_id, + SelectStep(absl::string_view field, bool test_field_presence, int64_t expr_id, bool enable_wrapper_type_null_unboxing, bool enable_optional_types) : ExpressionStepBase(expr_id), - field_value_(std::move(value)), - field_(field_value_.ToString()), + field_(field), test_field_presence_(test_field_presence), unboxing_option_(enable_wrapper_type_null_unboxing ? ProtoWrapperTypeOptions::kUnsetNull @@ -220,7 +219,6 @@ class SelectStep : public ExpressionStepBase { absl::Status Evaluate(ExecutionFrame* frame) const override; protected: - cel::StringValue field_value_; std::string field_; bool test_field_presence_; ProtoWrapperTypeOptions unboxing_option_; @@ -281,8 +279,9 @@ absl::Status SelectStep::Evaluate(ExecutionFrame* frame) const { target = &result; } CEL_RETURN_IF_ERROR( - PerformHas(*target, field_, field_value_, frame->descriptor_pool(), - frame->message_factory(), frame->arena(), result)); + PerformHas(*target, field_, cel::StringValue::WrapUnsafe(field_), + frame->descriptor_pool(), frame->message_factory(), + frame->arena(), result)); frame->value_stack().PopAndPush(std::move(result), std::move(result_trail)); return absl::OkStatus(); } @@ -296,8 +295,8 @@ absl::Status SelectStep::Evaluate(ExecutionFrame* frame) const { Value value; optional_arg->Value(&value); auto status = PerformOptionalGet( - value, field_, field_value_, unboxing_option_, frame->descriptor_pool(), - frame->message_factory(), frame->arena(), + value, field_, cel::StringValue::WrapUnsafe(field_), unboxing_option_, + frame->descriptor_pool(), frame->message_factory(), frame->arena(), frame->options().enable_use_new_field_select_implementation, result); if (!status.ok()) { result = ErrorValue(std::move(status)); @@ -307,8 +306,8 @@ absl::Status SelectStep::Evaluate(ExecutionFrame* frame) const { } CEL_RETURN_IF_ERROR(PerformGet( - arg, field_, field_value_, unboxing_option_, frame->descriptor_pool(), - frame->message_factory(), frame->arena(), + arg, field_, cel::StringValue::WrapUnsafe(field_), unboxing_option_, + frame->descriptor_pool(), frame->message_factory(), frame->arena(), frame->options().enable_use_new_field_select_implementation, result)); frame->value_stack().PopAndPush(std::move(result), std::move(result_trail)); return absl::OkStatus(); @@ -318,13 +317,12 @@ class DirectSelectStep : public DirectExpressionStep { public: DirectSelectStep(int64_t expr_id, std::unique_ptr operand, - StringValue field, bool test_only, + absl::string_view field, bool test_only, bool enable_wrapper_type_null_unboxing, bool enable_optional_types) : DirectExpressionStep(expr_id), operand_(std::move(operand)), - field_value_(std::move(field)), - field_(field_value_.ToString()), + field_(field), test_only_(test_only), unboxing_option_(enable_wrapper_type_null_unboxing ? ProtoWrapperTypeOptions::kUnsetNull @@ -375,11 +373,13 @@ class DirectSelectStep : public DirectExpressionStep { } Value value; optional_arg->Value(&value); - return PerformHas(value, field_, field_value_, frame.descriptor_pool(), - frame.message_factory(), frame.arena(), result); + return PerformHas(value, field_, cel::StringValue::WrapUnsafe(field_), + frame.descriptor_pool(), frame.message_factory(), + frame.arena(), result); } - return PerformHas(result, field_, field_value_, frame.descriptor_pool(), - frame.message_factory(), frame.arena(), result); + return PerformHas(result, field_, cel::StringValue::WrapUnsafe(field_), + frame.descriptor_pool(), frame.message_factory(), + frame.arena(), result); } if (optional_arg) { @@ -390,7 +390,7 @@ class DirectSelectStep : public DirectExpressionStep { Value value; optional_arg->Value(&value); auto status = PerformOptionalGet( - value, field_, field_value_, unboxing_option_, + value, field_, cel::StringValue::WrapUnsafe(field_), unboxing_option_, frame.descriptor_pool(), frame.message_factory(), frame.arena(), frame.options().enable_use_new_field_select_implementation, result); if (!status.ok()) { @@ -400,8 +400,8 @@ class DirectSelectStep : public DirectExpressionStep { } return PerformGet( - result, field_, field_value_, unboxing_option_, frame.descriptor_pool(), - frame.message_factory(), frame.arena(), + result, field_, cel::StringValue::WrapUnsafe(field_), unboxing_option_, + frame.descriptor_pool(), frame.message_factory(), frame.arena(), frame.options().enable_use_new_field_select_implementation, result); } @@ -413,7 +413,6 @@ class DirectSelectStep : public DirectExpressionStep { // // ToString or ValueManager::CreateString may force a copy so we do this at // plan time. - StringValue field_value_; std::string field_; // whether this is a has() expression. @@ -460,12 +459,12 @@ bool SupportsCachedFieldDescriptor( class ProtoSelectStep : public SelectStep { public: - ProtoSelectStep(StringValue value, int64_t expr_id, + ProtoSelectStep(absl::string_view value, int64_t expr_id, bool enable_wrapper_type_null_unboxing, bool enable_optional_types, const google::protobuf::Descriptor* descriptor, const google::protobuf::FieldDescriptor* field_descriptor) - : SelectStep(std::move(value), /*test_field_presence=*/false, expr_id, + : SelectStep(value, /*test_field_presence=*/false, expr_id, enable_wrapper_type_null_unboxing, enable_optional_types), descriptor_(descriptor), field_descriptor_(field_descriptor) { @@ -540,11 +539,11 @@ absl::Status ProtoSelectStep::EvaluateMessageFieldGet( class ProtoHasStep : public SelectStep { public: - ProtoHasStep(StringValue value, int64_t expr_id, + ProtoHasStep(absl::string_view field, int64_t expr_id, bool enable_wrapper_type_null_unboxing, bool enable_optional_types, const google::protobuf::Descriptor* descriptor, const google::protobuf::FieldDescriptor* field_descriptor) - : SelectStep(std::move(value), /*test_field_presence=*/true, expr_id, + : SelectStep(field, /*test_field_presence=*/true, expr_id, enable_wrapper_type_null_unboxing, enable_optional_types), descriptor_(descriptor), field_descriptor_(field_descriptor) { @@ -601,7 +600,7 @@ absl::Status ProtoHasStep::EvaluateHas( } // namespace std::unique_ptr CreateDirectSelectStep( - std::unique_ptr operand, StringValue field, + std::unique_ptr operand, absl::string_view field, bool test_only, int64_t expr_id, bool enable_wrapper_type_null_unboxing, bool enable_optional_types) { return std::make_unique( @@ -611,7 +610,7 @@ std::unique_ptr CreateDirectSelectStep( // Factory method for Select - based Execution step absl::StatusOr> CreateSelectStep( - cel::StringValue field, bool test_only, int64_t expr_id, + absl::string_view field, bool test_only, int64_t expr_id, bool enable_wrapper_type_null_unboxing, bool enable_optional_types) { return std::make_unique(std::move(field), test_only, expr_id, enable_wrapper_type_null_unboxing, @@ -620,7 +619,7 @@ absl::StatusOr> CreateSelectStep( // Factory method for Select - based Execution step absl::StatusOr> CreateTypedSelectStep( - cel::StringValue field, cel::StructType resolved_operand_type, + absl::string_view field, cel::StructType resolved_operand_type, cel::StructTypeField resolved_field, bool test_only, int64_t expr_id, bool enable_wrapper_type_null_unboxing, bool enable_optional_types) { if (!resolved_operand_type.IsMessage()) { diff --git a/eval/eval/select_step.h b/eval/eval/select_step.h index c3f965a94..17a93d19f 100644 --- a/eval/eval/select_step.h +++ b/eval/eval/select_step.h @@ -5,8 +5,8 @@ #include #include "absl/status/statusor.h" +#include "absl/strings/string_view.h" #include "common/type.h" -#include "common/value.h" #include "eval/eval/direct_expression_step.h" #include "eval/eval/evaluator_core.h" @@ -14,17 +14,17 @@ namespace google::api::expr::runtime { // Factory method for recursively evaluated select step. std::unique_ptr CreateDirectSelectStep( - std::unique_ptr operand, cel::StringValue field, + std::unique_ptr operand, absl::string_view field, bool test_only, int64_t expr_id, bool enable_wrapper_type_null_unboxing, bool enable_optional_types = false); // Factory method for Select stack machine based Execution step absl::StatusOr> CreateSelectStep( - cel::StringValue field, bool test_only, int64_t expr_id, + absl::string_view field, bool test_only, int64_t expr_id, bool enable_wrapper_type_null_unboxing, bool enable_optional_ytpes = false); absl::StatusOr> CreateTypedSelectStep( - cel::StringValue field, cel::StructType resolved_operand_type, + absl::string_view field, cel::StructType resolved_operand_type, cel::StructTypeField resolved_field, bool test_only, int64_t expr_id, bool enable_wrapper_type_null_unboxing, bool enable_optional_types); diff --git a/eval/eval/select_step_test.cc b/eval/eval/select_step_test.cc index 92cda2fbe..ddb15eb3d 100644 --- a/eval/eval/select_step_test.cc +++ b/eval/eval/select_step_test.cc @@ -107,8 +107,8 @@ class SelectStepTest : public testing::Test { CEL_ASSIGN_OR_RETURN(auto step0, CreateIdentStep(ident.name(), expr0.id())); CEL_ASSIGN_OR_RETURN( auto step1, - CreateSelectStep(cel::StringValue(select.field()), select.test_only(), - expr.id(), options.enable_wrapper_type_null_unboxing)); + CreateSelectStep(select.field(), select.test_only(), expr.id(), + options.enable_wrapper_type_null_unboxing)); path.push_back(std::move(step0)); path.push_back(std::move(step1)); @@ -290,13 +290,11 @@ TEST_F(SelectStepTest, MapPresenseIsErrorTest) { ASSERT_OK_AND_ASSIGN(auto step0, CreateIdentStep(ident.name(), expr0.id())); ASSERT_OK_AND_ASSIGN( auto step1, - CreateSelectStep(cel::StringValue(select_map.field()), - select_map.test_only(), expr1.id(), + CreateSelectStep(select_map.field(), select_map.test_only(), expr1.id(), /*enable_wrapper_type_null_unboxing=*/false)); ASSERT_OK_AND_ASSIGN( auto step2, - CreateSelectStep(cel::StringValue(select.field()), select.test_only(), - select_expr.id(), + CreateSelectStep(select.field(), select.test_only(), select_expr.id(), /*enable_wrapper_type_null_unboxing=*/false)); ExecutionPath path; @@ -752,8 +750,7 @@ TEST_P(SelectStepConformanceTest, CelErrorAsArgument) { ASSERT_OK_AND_ASSIGN(auto step0, CreateIdentStep(ident.name(), expr0.id())); ASSERT_OK_AND_ASSIGN( auto step1, - CreateSelectStep(cel::StringValue(select.field()), select.test_only(), - dummy_expr.id(), + CreateSelectStep(select.field(), select.test_only(), dummy_expr.id(), /*enable_wrapper_type_null_unboxing=*/false)); path.push_back(std::move(step0)); @@ -794,8 +791,7 @@ TEST_F(SelectStepTest, DisableMissingAttributeOK) { ASSERT_OK_AND_ASSIGN(auto step0, CreateIdentStep(ident.name(), expr0.id())); ASSERT_OK_AND_ASSIGN( auto step1, - CreateSelectStep(cel::StringValue(select.field()), select.test_only(), - dummy_expr.id(), + CreateSelectStep(select.field(), select.test_only(), dummy_expr.id(), /*enable_wrapper_type_null_unboxing=*/false)); path.push_back(std::move(step0)); @@ -837,8 +833,7 @@ TEST_F(SelectStepTest, UnrecoverableUnknownValueProducesError) { ASSERT_OK_AND_ASSIGN(auto step0, CreateIdentStep(ident.name(), expr0.id())); ASSERT_OK_AND_ASSIGN( auto step1, - CreateSelectStep(cel::StringValue(select.field()), select.test_only(), - dummy_expr.id(), + CreateSelectStep(select.field(), select.test_only(), dummy_expr.id(), /*enable_wrapper_type_null_unboxing=*/false)); path.push_back(std::move(step0)); @@ -884,9 +879,9 @@ TEST_F(SelectStepTest, UnknownPatternResolvesToUnknown) { auto& ident = expr0.mutable_ident_expr(); ident.set_name("message"); auto step0_status = CreateIdentStep(ident.name(), expr0.id()); - auto step1_status = CreateSelectStep( - cel::StringValue(select.field()), select.test_only(), dummy_expr.id(), - /*enable_wrapper_type_null_unboxing=*/false); + auto step1_status = + CreateSelectStep(select.field(), select.test_only(), dummy_expr.id(), + /*enable_wrapper_type_null_unboxing=*/false); ASSERT_THAT(step0_status, IsOk()); ASSERT_THAT(step1_status, IsOk()); @@ -987,12 +982,11 @@ TEST_P(SelectStepConformanceTest, TypedSelectStepTest) { cel::StructTypeField resolved_field((cel::MessageTypeField(field_desc))); ASSERT_OK_AND_ASSIGN( - auto step1, - CreateTypedSelectStep(cel::StringValue("single_int64"), - resolved_operand_type, resolved_field, - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/false, - /*enable_optional_types=*/false)); + auto step1, CreateTypedSelectStep( + "single_int64", resolved_operand_type, resolved_field, + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/false, + /*enable_optional_types=*/false)); ExecutionPath path; path.push_back(std::move(step0)); @@ -1028,12 +1022,11 @@ TEST_P(SelectStepConformanceTest, TypedSelectStepPropagatesUnknown) { cel::StructTypeField resolved_field((cel::MessageTypeField(field_desc))); ASSERT_OK_AND_ASSIGN( - auto step1, - CreateTypedSelectStep(cel::StringValue("single_int64"), - resolved_operand_type, resolved_field, - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/false, - /*enable_optional_types=*/false)); + auto step1, CreateTypedSelectStep( + "single_int64", resolved_operand_type, resolved_field, + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/false, + /*enable_optional_types=*/false)); ExecutionPath path; path.push_back(std::move(step0)); @@ -1066,12 +1059,11 @@ TEST_F(SelectStepTest, TypedSelectStepUnknownPatternResolvesToUnknown) { cel::StructTypeField resolved_field((cel::MessageTypeField(field_desc))); ASSERT_OK_AND_ASSIGN( - auto step1, - CreateTypedSelectStep(cel::StringValue("single_int64"), - resolved_operand_type, resolved_field, - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/false, - /*enable_optional_types=*/false)); + auto step1, CreateTypedSelectStep( + "single_int64", resolved_operand_type, resolved_field, + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/false, + /*enable_optional_types=*/false)); ExecutionPath path; path.push_back(std::move(step0)); @@ -1133,14 +1125,18 @@ TEST_F(DirectSelectStepTest, SelectFromMap) { cel::Activation activation; RuntimeOptions options; - auto step = CreateDirectSelectStep( - CreateDirectIdentStep("map_val", -1), cel::StringValue("one"), - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/true); + auto step = + CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), "one", + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/true); auto map_builder = cel::NewMapValueBuilder(&arena_); - ASSERT_THAT(map_builder->Put(cel::StringValue("one"), IntValue(1)), IsOk()); - ASSERT_THAT(map_builder->Put(cel::StringValue("two"), IntValue(2)), IsOk()); + ASSERT_THAT( + map_builder->Put(cel::StringValue::WrapUnsafe("one"), IntValue(1)), + IsOk()); + ASSERT_THAT( + map_builder->Put(cel::StringValue::WrapUnsafe("two"), IntValue(2)), + IsOk()); activation.InsertOrAssignValue("map_val", std::move(*map_builder).Build()); ExecutionFrameBase frame(activation, options, type_provider_, @@ -1160,14 +1156,18 @@ TEST_F(DirectSelectStepTest, HasMap) { cel::Activation activation; RuntimeOptions options; - auto step = CreateDirectSelectStep( - CreateDirectIdentStep("map_val", -1), cel::StringValue("two"), - /*test_only=*/true, -1, - /*enable_wrapper_type_null_unboxing=*/true); + auto step = + CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), "two", + /*test_only=*/true, -1, + /*enable_wrapper_type_null_unboxing=*/true); auto map_builder = cel::NewMapValueBuilder(&arena_); - ASSERT_THAT(map_builder->Put(cel::StringValue("one"), IntValue(1)), IsOk()); - ASSERT_THAT(map_builder->Put(cel::StringValue("two"), IntValue(2)), IsOk()); + ASSERT_THAT( + map_builder->Put(cel::StringValue::WrapUnsafe("one"), IntValue(1)), + IsOk()); + ASSERT_THAT( + map_builder->Put(cel::StringValue::WrapUnsafe("two"), IntValue(2)), + IsOk()); activation.InsertOrAssignValue("map_val", std::move(*map_builder).Build()); ExecutionFrameBase frame(activation, options, type_provider_, @@ -1187,15 +1187,19 @@ TEST_F(DirectSelectStepTest, SelectFromOptionalMap) { cel::Activation activation; RuntimeOptions options; - auto step = CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), - cel::StringValue("one"), - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/true, - /*enable_optional_types=*/true); + auto step = + CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), "one", + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/true, + /*enable_optional_types=*/true); auto map_builder = cel::NewMapValueBuilder(&arena_); - ASSERT_THAT(map_builder->Put(cel::StringValue("one"), IntValue(1)), IsOk()); - ASSERT_THAT(map_builder->Put(cel::StringValue("two"), IntValue(2)), IsOk()); + ASSERT_THAT( + map_builder->Put(cel::StringValue::WrapUnsafe("one"), IntValue(1)), + IsOk()); + ASSERT_THAT( + map_builder->Put(cel::StringValue::WrapUnsafe("two"), IntValue(2)), + IsOk()); activation.InsertOrAssignValue( "map_val", OptionalValue::Of(std::move(*map_builder).Build(), &arena_)); @@ -1216,15 +1220,19 @@ TEST_F(DirectSelectStepTest, SelectFromOptionalMapAbsent) { cel::Activation activation; RuntimeOptions options; - auto step = CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), - cel::StringValue("three"), - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/true, - /*enable_optional_types=*/true); + auto step = + CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), "three", + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/true, + /*enable_optional_types=*/true); auto map_builder = cel::NewMapValueBuilder(&arena_); - ASSERT_THAT(map_builder->Put(cel::StringValue("one"), IntValue(1)), IsOk()); - ASSERT_THAT(map_builder->Put(cel::StringValue("two"), IntValue(2)), IsOk()); + ASSERT_THAT( + map_builder->Put(cel::StringValue::WrapUnsafe("one"), IntValue(1)), + IsOk()); + ASSERT_THAT( + map_builder->Put(cel::StringValue::WrapUnsafe("two"), IntValue(2)), + IsOk()); activation.InsertOrAssignValue( "map_val", OptionalValue::Of(std::move(*map_builder).Build(), &arena_)); @@ -1246,7 +1254,7 @@ TEST_F(DirectSelectStepTest, SelectFromOptionalStruct) { RuntimeOptions options; auto step = CreateDirectSelectStep(CreateDirectIdentStep("struct_val", -1), - cel::StringValue("single_int64"), + "single_int64", /*test_only=*/false, -1, /*enable_wrapper_type_null_unboxing=*/true, /*enable_optional_types=*/true); @@ -1281,7 +1289,7 @@ TEST_F(DirectSelectStepTest, SelectFromOptionalStructFieldNotSet) { RuntimeOptions options; auto step = CreateDirectSelectStep(CreateDirectIdentStep("struct_val", -1), - cel::StringValue("single_string"), + "single_string", /*test_only=*/false, -1, /*enable_wrapper_type_null_unboxing=*/true, /*enable_optional_types=*/true); @@ -1315,11 +1323,11 @@ TEST_F(DirectSelectStepTest, SelectFromEmptyOptional) { cel::Activation activation; RuntimeOptions options; - auto step = CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), - cel::StringValue("one"), - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/true, - /*enable_optional_types=*/true); + auto step = + CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), "one", + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/true, + /*enable_optional_types=*/true); activation.InsertOrAssignValue("map_val", OptionalValue::None()); @@ -1340,15 +1348,19 @@ TEST_F(DirectSelectStepTest, HasOptional) { cel::Activation activation; RuntimeOptions options; - auto step = CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), - cel::StringValue("two"), - /*test_only=*/true, -1, - /*enable_wrapper_type_null_unboxing=*/true, - /*enable_optional_types=*/true); + auto step = + CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), "two", + /*test_only=*/true, -1, + /*enable_wrapper_type_null_unboxing=*/true, + /*enable_optional_types=*/true); auto map_builder = cel::NewMapValueBuilder(&arena_); - ASSERT_THAT(map_builder->Put(cel::StringValue("one"), IntValue(1)), IsOk()); - ASSERT_THAT(map_builder->Put(cel::StringValue("two"), IntValue(2)), IsOk()); + ASSERT_THAT( + map_builder->Put(cel::StringValue::WrapUnsafe("one"), IntValue(1)), + IsOk()); + ASSERT_THAT( + map_builder->Put(cel::StringValue::WrapUnsafe("two"), IntValue(2)), + IsOk()); activation.InsertOrAssignValue( "map_val", OptionalValue::Of(std::move(*map_builder).Build(), &arena_)); @@ -1369,11 +1381,11 @@ TEST_F(DirectSelectStepTest, HasEmptyOptional) { cel::Activation activation; RuntimeOptions options; - auto step = CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), - cel::StringValue("two"), - /*test_only=*/true, -1, - /*enable_wrapper_type_null_unboxing=*/true, - /*enable_optional_types=*/true); + auto step = + CreateDirectSelectStep(CreateDirectIdentStep("map_val", -1), "two", + /*test_only=*/true, -1, + /*enable_wrapper_type_null_unboxing=*/true, + /*enable_optional_types=*/true); activation.InsertOrAssignValue("map_val", OptionalValue::None()); @@ -1394,11 +1406,10 @@ TEST_F(DirectSelectStepTest, SelectFromStruct) { cel::Activation activation; RuntimeOptions options; - auto step = - CreateDirectSelectStep(CreateDirectIdentStep("test_all_types", -1), - cel::StringValue("single_int64"), - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/true); + auto step = CreateDirectSelectStep( + CreateDirectIdentStep("test_all_types", -1), "single_int64", + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/true); TestAllTypes message; message.set_single_int64(1); @@ -1421,11 +1432,10 @@ TEST_F(DirectSelectStepTest, HasStruct) { cel::Activation activation; RuntimeOptions options; - auto step = - CreateDirectSelectStep(CreateDirectIdentStep("test_all_types", -1), - cel::StringValue("single_string"), - /*test_only=*/true, -1, - /*enable_wrapper_type_null_unboxing=*/true); + auto step = CreateDirectSelectStep( + CreateDirectIdentStep("test_all_types", -1), "single_string", + /*test_only=*/true, -1, + /*enable_wrapper_type_null_unboxing=*/true); TestAllTypes message; message.set_single_int64(1); @@ -1449,10 +1459,10 @@ TEST_F(DirectSelectStepTest, SelectFromUnsupportedType) { cel::Activation activation; RuntimeOptions options; - auto step = CreateDirectSelectStep( - CreateDirectIdentStep("bool_val", -1), cel::StringValue("one"), - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/true); + auto step = + CreateDirectSelectStep(CreateDirectIdentStep("bool_val", -1), "one", + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/true); activation.InsertOrAssignValue("bool_val", BoolValue(false)); @@ -1476,11 +1486,10 @@ TEST_F(DirectSelectStepTest, AttributeUpdatedIfRequested) { RuntimeOptions options; options.unknown_processing = cel::UnknownProcessingOptions::kAttributeOnly; - auto step = - CreateDirectSelectStep(CreateDirectIdentStep("test_all_types", -1), - cel::StringValue("single_int64"), - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/true); + auto step = CreateDirectSelectStep( + CreateDirectIdentStep("test_all_types", -1), "single_int64", + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/true); TestAllTypes message; message.set_single_int64(1); @@ -1506,11 +1515,10 @@ TEST_F(DirectSelectStepTest, MissingAttributesToErrors) { RuntimeOptions options; options.enable_missing_attribute_errors = true; - auto step = - CreateDirectSelectStep(CreateDirectIdentStep("test_all_types", -1), - cel::StringValue("single_int64"), - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/true); + auto step = CreateDirectSelectStep( + CreateDirectIdentStep("test_all_types", -1), "single_int64", + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/true); TestAllTypes message; message.set_single_int64(1); @@ -1538,11 +1546,10 @@ TEST_F(DirectSelectStepTest, IdentifiesUnknowns) { RuntimeOptions options; options.unknown_processing = cel::UnknownProcessingOptions::kAttributeOnly; - auto step = - CreateDirectSelectStep(CreateDirectIdentStep("test_all_types", -1), - cel::StringValue("single_int64"), - /*test_only=*/false, -1, - /*enable_wrapper_type_null_unboxing=*/true); + auto step = CreateDirectSelectStep( + CreateDirectIdentStep("test_all_types", -1), "single_int64", + /*test_only=*/false, -1, + /*enable_wrapper_type_null_unboxing=*/true); TestAllTypes message; message.set_single_int64(1); @@ -1573,7 +1580,7 @@ TEST_F(DirectSelectStepTest, ForwardErrorValue) { auto step = CreateDirectSelectStep( CreateConstValueDirectStep(cel::ErrorValue(absl::InternalError("test1")), -1), - cel::StringValue("single_int64"), + "single_int64", /*test_only=*/false, -1, /*enable_wrapper_type_null_unboxing=*/true); @@ -1599,7 +1606,7 @@ TEST_F(DirectSelectStepTest, ForwardUnknownOperand) { auto step = CreateDirectSelectStep( CreateConstValueDirectStep( cel::UnknownValue(cel::Unknown(std::move(attr_set))), -1), - cel::StringValue("single_int64"), + "single_int64", /*test_only=*/false, -1, /*enable_wrapper_type_null_unboxing=*/true); diff --git a/eval/tests/modern_benchmark_test.cc b/eval/tests/modern_benchmark_test.cc index 8a41b094f..1d7731e6a 100644 --- a/eval/tests/modern_benchmark_test.cc +++ b/eval/tests/modern_benchmark_test.cc @@ -383,9 +383,9 @@ void BM_PolicySymbolic(benchmark::State& state) { ASSERT_OK_AND_ASSIGN(auto cel_expr, runtime->CreateProgram(std::move(ast))); Activation activation; - activation.InsertOrAssignValue("ip", StringValue(&arena, kIP)); - activation.InsertOrAssignValue("path", StringValue(&arena, kPath)); - activation.InsertOrAssignValue("token", StringValue(&arena, kToken)); + activation.InsertOrAssignValue("ip", StringValue::WrapUnsafe(kIP)); + activation.InsertOrAssignValue("path", StringValue::WrapUnsafe(kPath)); + activation.InsertOrAssignValue("token", StringValue::WrapUnsafe(kToken)); for (auto _ : state) { ASSERT_OK_AND_ASSIGN(cel::Value result, @@ -439,11 +439,11 @@ class RequestMapImpl : public CustomMapValueInterface { return false; } if (string_value->Equals("ip")) { - *result = StringValue(kIP); + *result = StringValue::WrapUnsafe(kIP); } else if (string_value->Equals("path")) { - *result = StringValue(kPath); + *result = StringValue::WrapUnsafe(kPath); } else if (string_value->Equals("token")) { - *result = StringValue(kToken); + *result = StringValue::WrapUnsafe(kToken); } else { return false; } @@ -684,9 +684,9 @@ void BM_HasMap(benchmark::State& state) { auto map_builder = cel::NewMapValueBuilder(&arena); - ASSERT_THAT( - map_builder->Put(cel::StringValue("path"), cel::StringValue("path")), - IsOk()); + ASSERT_THAT(map_builder->Put(cel::StringValue::WrapUnsafe("path"), + cel::StringValue::WrapUnsafe("path")), + IsOk()); activation.InsertOrAssignValue("request", std::move(*map_builder).Build()); diff --git a/extensions/encoders.cc b/extensions/encoders.cc index 66431b30b..fdcf4dd59 100644 --- a/extensions/encoders.cc +++ b/extensions/encoders.cc @@ -50,7 +50,7 @@ absl::StatusOr Base64Decode( if (!absl::Base64Unescape(value.NativeString(in), &out)) { return ErrorValue{absl::InvalidArgumentError("invalid base64 data")}; } - return BytesValue(arena, std::move(out)); + return BytesValue::From(std::move(out), arena); } absl::StatusOr Base64Encode( @@ -61,7 +61,7 @@ absl::StatusOr Base64Encode( std::string in; std::string out; out = absl::Base64Escape(value.NativeString(in)); - return StringValue(arena, std::move(out)); + return StringValue::From(std::move(out), arena); } absl::Status RegisterEncodersDecls(TypeCheckerBuilder& builder) { diff --git a/extensions/formatting_test.cc b/extensions/formatting_test.cc index 6a7fb300b..a07dd0bb1 100644 --- a/extensions/formatting_test.cc +++ b/extensions/formatting_test.cc @@ -182,8 +182,8 @@ TEST_P(StringFormatTest, TestStringFormatting) { Activation activation; for (const auto& [name, value] : test_case.dyn_args) { if (std::holds_alternative(value)) { - activation.InsertOrAssignValue(name, - StringValue{std::get(value)}); + activation.InsertOrAssignValue( + name, StringValue::From(std::get(value), &arena)); } else if (std::holds_alternative(value)) { activation.InsertOrAssignValue(name, BoolValue{std::get(value)}); } else if (std::holds_alternative(value)) { diff --git a/extensions/protobuf/value_test.cc b/extensions/protobuf/value_test.cc index 79c15690e..b43c65968 100644 --- a/extensions/protobuf/value_test.cc +++ b/extensions/protobuf/value_test.cc @@ -841,8 +841,9 @@ TEST_F(ProtoValueUnwrapTest, NonMessageValue) { TestAllTypes dest; EXPECT_THAT(ProtoMessageFromValue(IntValue(42), dest), StatusIs(absl::StatusCode::kInvalidArgument)); - EXPECT_THAT(ProtoMessageFromValue(StringValue("not a message"), dest), - StatusIs(absl::StatusCode::kInvalidArgument)); + EXPECT_THAT( + ProtoMessageFromValue(StringValue::WrapUnsafe("not a message"), dest), + StatusIs(absl::StatusCode::kInvalidArgument)); EXPECT_THAT(ProtoMessageFromValue(NullValue(), dest), StatusIs(absl::StatusCode::kInvalidArgument)); } diff --git a/extensions/select_optimization.cc b/extensions/select_optimization.cc index 4dcd7d594..d3961d9a1 100644 --- a/extensions/select_optimization.cc +++ b/extensions/select_optimization.cc @@ -420,7 +420,7 @@ absl::StatusOr FallbackSelect( } return elem->GetMap().Has( - StringValue(arena, *qualifier.GetStringKey()), + StringValue::WrapUnsafe(*qualifier.GetStringKey()), descriptor_pool, message_factory, arena); }), last_instruction); diff --git a/extensions/select_optimization_test.cc b/extensions/select_optimization_test.cc index 27c191738..c5bae0db9 100644 --- a/extensions/select_optimization_test.cc +++ b/extensions/select_optimization_test.cc @@ -1105,8 +1105,8 @@ INSTANTIATE_TEST_SUITE_P( {}, // not set [](google::protobuf::Arena* arena, Activation& act) -> absl::Status { auto builder = cel::NewMapValueBuilder(arena); - CEL_RETURN_IF_ERROR( - builder->Put(cel::StringValue("child"), cel::NullValue())); + CEL_RETURN_IF_ERROR(builder->Put( + cel::StringValue::WrapUnsafe("child"), cel::NullValue())); auto value = std::move(*builder).Build(); diff --git a/runtime/constant_folding_test.cc b/runtime/constant_folding_test.cc index c59d5602a..0770039f3 100644 --- a/runtime/constant_folding_test.cc +++ b/runtime/constant_folding_test.cc @@ -87,9 +87,11 @@ TEST_P(ConstantFoldingExtTest, Runner) { const StringValue&>:: RegisterGlobalOverload( "prepend", - [](const StringValue& value, const StringValue& prefix) { - return StringValue( - absl::StrCat(prefix.ToString(), value.ToString())); + [](const StringValue& value, const StringValue& prefix, + const Function::InvokeContext& context) { + return StringValue::From( + absl::StrCat(prefix.ToString(), value.ToString()), + context.arena()); }, builder.function_registry()); ASSERT_THAT(status, IsOk()); @@ -150,9 +152,11 @@ TEST(ConstantFoldingExtTest, LazyFunctionNotFolded) { BinaryFunctionAdapter, const StringValue&, const StringValue&>; auto fn = FunctionAdapter::WrapFunction( - [&call_count](const StringValue& value, const StringValue& prefix) { + [&call_count](const StringValue& value, const StringValue& prefix, + const Function::InvokeContext& context) { call_count++; - return StringValue(absl::StrCat(prefix.ToString(), value.ToString())); + return StringValue::From( + absl::StrCat(prefix.ToString(), value.ToString()), context.arena()); }); FunctionDescriptor descriptor = FunctionAdapter::CreateDescriptor( "lazy_prepend", /*receiver_style=*/false); @@ -189,19 +193,21 @@ TEST(ConstantFoldingExtTest, ContextualFunctionNotFolded) { internal::GetTestingDescriptorPool(), options)); int call_count = 0; - auto status = BinaryFunctionAdapter< - absl::StatusOr, const StringValue&, - const StringValue&>::Register("contextual_prepend", - /*receiver_style=*/false, - [&call_count](const StringValue& value, - const StringValue& prefix) { - call_count++; - return StringValue(absl::StrCat( - prefix.ToString(), value.ToString())); - }, - builder.function_registry(), - {/*.is_strict=*/true, - /*is_contextual=*/true}); + auto status = BinaryFunctionAdapter, const StringValue&, + const StringValue&>:: + Register( + "contextual_prepend", + /*receiver_style=*/false, + [&call_count](const StringValue& value, const StringValue& prefix, + const Function::InvokeContext& context) { + call_count++; + return StringValue::From( + absl::StrCat(prefix.ToString(), value.ToString()), + context.arena()); + }, + builder.function_registry(), + {/*.is_strict=*/true, + /*is_contextual=*/true}); ASSERT_THAT(status, IsOk()); ASSERT_THAT(EnableConstantFolding(builder), IsOk()); diff --git a/runtime/function_adapter_test.cc b/runtime/function_adapter_test.cc index 910020fdf..df5f50362 100644 --- a/runtime/function_adapter_test.cc +++ b/runtime/function_adapter_test.cc @@ -162,13 +162,14 @@ TEST_F(FunctionAdapterTest, UnaryFunctionAdapterWrapFunctionDuration) { TEST_F(FunctionAdapterTest, UnaryFunctionAdapterWrapFunctionString) { using FunctionAdapter = UnaryFunctionAdapter; - std::unique_ptr wrapped = - FunctionAdapter::WrapFunction([](const StringValue& x) -> StringValue { - return StringValue("pre_" + x.ToString()); + std::unique_ptr wrapped = FunctionAdapter::WrapFunction( + [](const StringValue& x, + const Function::InvokeContext& context) -> StringValue { + return StringValue::From("pre_" + x.ToString(), context.arena()); }); std::vector args; - args.emplace_back() = StringValue("string"); + args.emplace_back() = StringValue::WrapUnsafe("string"); ASSERT_OK_AND_ASSIGN(auto result, wrapped->Invoke(args, test_invoke_context())); @@ -178,13 +179,14 @@ TEST_F(FunctionAdapterTest, UnaryFunctionAdapterWrapFunctionString) { TEST_F(FunctionAdapterTest, UnaryFunctionAdapterWrapFunctionBytes) { using FunctionAdapter = UnaryFunctionAdapter; - std::unique_ptr wrapped = - FunctionAdapter::WrapFunction([](const BytesValue& x) -> BytesValue { - return BytesValue("pre_" + x.ToString()); + std::unique_ptr wrapped = FunctionAdapter::WrapFunction( + [](const BytesValue& x, + const Function::InvokeContext& context) -> BytesValue { + return BytesValue::From("pre_" + x.ToString(), context.arena()); }); std::vector args; - args.emplace_back() = BytesValue("bytes"); + args.emplace_back() = BytesValue::WrapUnsafe("bytes"); ASSERT_OK_AND_ASSIGN(auto result, wrapped->Invoke(args, test_invoke_context())); @@ -480,14 +482,15 @@ TEST_F(FunctionAdapterTest, BinaryFunctionAdapterWrapFunctionString) { BinaryFunctionAdapter, const StringValue&, const StringValue&>; std::unique_ptr wrapped = FunctionAdapter::WrapFunction( - [](const StringValue& x, - const StringValue& y) -> absl::StatusOr { - return StringValue(x.ToString() + y.ToString()); + [](const StringValue& x, const StringValue& y, + const Function::InvokeContext& context) + -> absl::StatusOr { + return StringValue::From(x.ToString() + y.ToString(), context.arena()); }); std::vector args; - args.emplace_back() = StringValue("abc"); - args.emplace_back() = StringValue("def"); + args.emplace_back() = StringValue::WrapUnsafe("abc"); + args.emplace_back() = StringValue::WrapUnsafe("def"); ASSERT_OK_AND_ASSIGN(auto result, wrapped->Invoke(args, test_invoke_context())); @@ -501,14 +504,14 @@ TEST_F(FunctionAdapterTest, BinaryFunctionAdapterWrapFunctionBytes) { BinaryFunctionAdapter, const BytesValue&, const BytesValue&>; std::unique_ptr wrapped = FunctionAdapter::WrapFunction( - [](const BytesValue& x, - const BytesValue& y) -> absl::StatusOr { - return BytesValue(x.ToString() + y.ToString()); + [](const BytesValue& x, const BytesValue& y, + const Function::InvokeContext& context) -> absl::StatusOr { + return BytesValue::From(x.ToString() + y.ToString(), context.arena()); }); std::vector args; - args.emplace_back() = BytesValue("abc"); - args.emplace_back() = BytesValue("def"); + args.emplace_back() = BytesValue::WrapUnsafe("abc"); + args.emplace_back() = BytesValue::WrapUnsafe("def"); ASSERT_OK_AND_ASSIGN(auto result, wrapped->Invoke(args, test_invoke_context())); @@ -712,7 +715,7 @@ TEST_F(FunctionAdapterTest, NaryFunctionAdapterCreateDescriptor0Args) { TEST_F(FunctionAdapterTest, NaryFunctionAdapterWrapFunction0Args) { std::unique_ptr fn = NullaryFunctionAdapter>::WrapFunction( - []() { return StringValue("abc"); }); + []() { return StringValue::WrapUnsafe("abc"); }); ASSERT_OK_AND_ASSIGN(auto result, fn->Invoke({}, descriptor_pool(), message_factory(), arena())); @@ -734,16 +737,18 @@ TEST_F(FunctionAdapterTest, NaryFunctionAdapterCreateDescriptor3Args) { TEST_F(FunctionAdapterTest, NaryFunctionAdapterWrapFunction3Args) { std::unique_ptr fn = NaryFunctionAdapter< - absl::StatusOr, int64_t, bool, - const StringValue&>::WrapFunction([](int64_t int_val, bool bool_val, - const StringValue& string_val) - -> absl::StatusOr { - return StringValue(absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), - "_", string_val.ToString())); - }); + absl::StatusOr, int64_t, bool, const StringValue&>:: + WrapFunction( + [](int64_t int_val, bool bool_val, const StringValue& string_val, + const Function::InvokeContext& context) -> absl::StatusOr { + return StringValue::From( + absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), "_", + string_val.ToString()), + context.arena()); + }); std::vector args{IntValue(42), BoolValue(false)}; - args.emplace_back() = StringValue("abcd"); + args.emplace_back() = StringValue::WrapUnsafe("abcd"); ASSERT_OK_AND_ASSIGN(auto result, fn->Invoke(args, descriptor_pool(), message_factory(), arena())); ASSERT_TRUE(result->Is()); @@ -752,13 +757,15 @@ TEST_F(FunctionAdapterTest, NaryFunctionAdapterWrapFunction3Args) { TEST_F(FunctionAdapterTest, NaryFunctionAdapterWrapFunction3ArgsBadArgType) { std::unique_ptr fn = NaryFunctionAdapter< - absl::StatusOr, int64_t, bool, - const StringValue&>::WrapFunction([](int64_t int_val, bool bool_val, - const StringValue& string_val) - -> absl::StatusOr { - return StringValue(absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), - "_", string_val.ToString())); - }); + absl::StatusOr, int64_t, bool, const StringValue&>:: + WrapFunction( + [](int64_t int_val, bool bool_val, const StringValue& string_val, + const Function::InvokeContext& context) -> absl::StatusOr { + return StringValue::From( + absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), "_", + string_val.ToString()), + context.arena()); + }); std::vector args{IntValue(42), BoolValue(false)}; args.emplace_back() = TimestampValue(absl::UnixEpoch()); @@ -769,13 +776,15 @@ TEST_F(FunctionAdapterTest, NaryFunctionAdapterWrapFunction3ArgsBadArgType) { TEST_F(FunctionAdapterTest, NaryFunctionAdapterWrapFunction3ArgsBadArgCount) { std::unique_ptr fn = NaryFunctionAdapter< - absl::StatusOr, int64_t, bool, - const StringValue&>::WrapFunction([](int64_t int_val, bool bool_val, - const StringValue& string_val) - -> absl::StatusOr { - return StringValue(absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), - "_", string_val.ToString())); - }); + absl::StatusOr, int64_t, bool, const StringValue&>:: + WrapFunction( + [](int64_t int_val, bool bool_val, const StringValue& string_val, + const Function::InvokeContext& context) -> absl::StatusOr { + return StringValue::From( + absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), "_", + string_val.ToString()), + context.arena()); + }); std::vector args{IntValue(42), BoolValue(false)}; EXPECT_THAT(fn->Invoke(args, test_invoke_context()), @@ -802,15 +811,17 @@ TEST_F(FunctionAdapterTest, NaryFunctionAdapterWrapFunction5Args) { absl::StatusOr, int64_t, bool, const StringValue&, int64_t, int64_t>::WrapFunction([](int64_t int_val, bool bool_val, const StringValue& string_val, - int64_t extra_arg, - int64_t extra_arg2) -> absl::StatusOr { - return StringValue(absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), - "_", string_val.ToString(), "_", extra_arg, - "_", extra_arg2)); + int64_t extra_arg, int64_t extra_arg2, + const Function::InvokeContext& context) + -> absl::StatusOr { + return StringValue::From( + absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), "_", + string_val.ToString(), "_", extra_arg, "_", extra_arg2), + context.arena()); }); std::vector args{IntValue(42), BoolValue(false)}; - args.emplace_back() = StringValue("abcd"); + args.emplace_back() = StringValue::WrapUnsafe("abcd"); args.push_back(IntValue(123)); args.push_back(IntValue(456)); ASSERT_OK_AND_ASSIGN(auto result, fn->Invoke(args, descriptor_pool(), @@ -824,12 +835,15 @@ TEST_F(FunctionAdapterTest, NaryFunctionAdapterWrapFunction5ArgsBadArgType) { absl::StatusOr, int64_t, bool, const StringValue&, int64_t, int64_t>::WrapFunction([](int64_t int_val, bool bool_val, const StringValue& string_val, - int64_t extra_arg, - int64_t extra_arg2) -> absl::StatusOr { + int64_t extra_arg, int64_t extra_arg2, + const Function::InvokeContext& context) + -> absl::StatusOr { static_cast(extra_arg); static_cast(extra_arg2); - return StringValue(absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), - "_", string_val.ToString())); + return StringValue::From( + absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), "_", + string_val.ToString()), + context.arena()); }); std::vector args{IntValue(42), BoolValue(false)}; @@ -846,12 +860,15 @@ TEST_F(FunctionAdapterTest, NaryFunctionAdapterWrapFunction5ArgsBadArgCount) { absl::StatusOr, int64_t, bool, const StringValue&, int64_t, int64_t>::WrapFunction([](int64_t int_val, bool bool_val, const StringValue& string_val, - int64_t extra_arg, - int64_t extra_arg2) -> absl::StatusOr { + int64_t extra_arg, int64_t extra_arg2, + const Function::InvokeContext& context) + -> absl::StatusOr { static_cast(extra_arg); static_cast(extra_arg2); - return StringValue(absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), - "_", string_val.ToString())); + return StringValue::From( + absl::StrCat(int_val, "_", (bool_val ? "true" : "false"), "_", + string_val.ToString()), + context.arena()); }); std::vector args{IntValue(42), BoolValue(false)}; diff --git a/runtime/internal/BUILD b/runtime/internal/BUILD index d8a442f34..f90d24f30 100644 --- a/runtime/internal/BUILD +++ b/runtime/internal/BUILD @@ -87,15 +87,15 @@ cc_library( srcs = ["convert_constant.cc"], hdrs = ["convert_constant.h"], deps = [ - "//common:allocator", - "//common:ast", "//common:constant", "//common:value", "//eval/internal:errors", + "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/time", "@com_google_absl//absl/types:variant", + "@com_google_protobuf//:protobuf", ], ) diff --git a/runtime/internal/convert_constant.cc b/runtime/internal/convert_constant.cc index 33f382858..832b0dfcc 100644 --- a/runtime/internal/convert_constant.cc +++ b/runtime/internal/convert_constant.cc @@ -17,21 +17,22 @@ #include #include +#include "absl/base/nullability.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/time/time.h" #include "absl/types/variant.h" -#include "common/allocator.h" #include "common/constant.h" #include "common/value.h" #include "eval/internal/errors.h" +#include "google/protobuf/arena.h" namespace cel::runtime_internal { namespace { using ::cel::Constant; struct ConvertVisitor { - Allocator<> allocator; + google::protobuf::Arena* const arena; absl::StatusOr operator()(std::monostate) { return absl::InvalidArgumentError("unspecified constant"); @@ -48,10 +49,10 @@ struct ConvertVisitor { return DoubleValue(value); } absl::StatusOr operator()(const cel::StringConstant& value) { - return StringValue(allocator, value); + return StringValue::From(value, arena); } absl::StatusOr operator()(const cel::BytesConstant& value) { - return BytesValue(allocator, value); + return BytesValue::From(value, arena); } absl::StatusOr operator()(const absl::Duration duration) { if (duration >= kDurationHigh || duration <= kDurationLow) { @@ -71,8 +72,8 @@ struct ConvertVisitor { // // A status maybe returned if value creation fails. absl::StatusOr ConvertConstant(const Constant& constant, - Allocator<> allocator) { - return absl::visit(ConvertVisitor{allocator}, constant.constant_kind()); + google::protobuf::Arena* absl_nonnull arena) { + return absl::visit(ConvertVisitor{arena}, constant.constant_kind()); } } // namespace cel::runtime_internal diff --git a/runtime/internal/convert_constant.h b/runtime/internal/convert_constant.h index f1ac0c850..e891ca45d 100644 --- a/runtime/internal/convert_constant.h +++ b/runtime/internal/convert_constant.h @@ -14,10 +14,11 @@ #ifndef THIRD_PARTY_CEL_CPP_RUNTIME_INTERNAL_CONVERT_CONSTANT_H_ #define THIRD_PARTY_CEL_CPP_RUNTIME_INTERNAL_CONVERT_CONSTANT_H_ +#include "absl/base/nullability.h" #include "absl/status/statusor.h" -#include "common/allocator.h" -#include "common/ast.h" +#include "common/constant.h" #include "common/value.h" +#include "google/protobuf/arena.h" namespace cel::runtime_internal { @@ -32,7 +33,7 @@ namespace cel::runtime_internal { // A status may still be returned if value creation fails according to // value_factory's policy. absl::StatusOr ConvertConstant(const Constant& constant, - Allocator<> allocator); + google::protobuf::Arena* absl_nonnull arena); } // namespace cel::runtime_internal diff --git a/runtime/internal/function_adapter_test.cc b/runtime/internal/function_adapter_test.cc index 643f08090..d57620999 100644 --- a/runtime/internal/function_adapter_test.cc +++ b/runtime/internal/function_adapter_test.cc @@ -179,7 +179,7 @@ TEST_F(ValueToAdaptedVisitorTest, DurationWrongKind) { } TEST_F(ValueToAdaptedVisitorTest, String) { - Value v = cel::StringValue("string"); + Value v = cel::StringValue::WrapUnsafe("string"); StringValue out; ASSERT_THAT(ValueToAdaptedVisitor{v}(&out), IsOk()); @@ -197,7 +197,7 @@ TEST_F(ValueToAdaptedVisitorTest, StringWrongKind) { } TEST_F(ValueToAdaptedVisitorTest, Bytes) { - Value v = cel::BytesValue("bytes"); + Value v = cel::BytesValue::WrapUnsafe("bytes"); BytesValue out; ASSERT_THAT(ValueToAdaptedVisitor{v}(&out), IsOk()); @@ -272,7 +272,7 @@ TEST_F(AdaptedToValueVisitorTest, Duration) { } TEST_F(AdaptedToValueVisitorTest, String) { - StringValue value = cel::StringValue("str"); + StringValue value = cel::StringValue::WrapUnsafe("str"); ASSERT_OK_AND_ASSIGN(auto result, AdaptedToValueVisitor{}(value)); @@ -281,7 +281,7 @@ TEST_F(AdaptedToValueVisitorTest, String) { } TEST_F(AdaptedToValueVisitorTest, Bytes) { - BytesValue value = cel::BytesValue("bytes"); + BytesValue value = cel::BytesValue::WrapUnsafe("bytes"); ASSERT_OK_AND_ASSIGN(auto result, AdaptedToValueVisitor{}(value)); diff --git a/runtime/regex_precompilation_test.cc b/runtime/regex_precompilation_test.cc index 85b47ef45..f31d34455 100644 --- a/runtime/regex_precompilation_test.cc +++ b/runtime/regex_precompilation_test.cc @@ -87,9 +87,11 @@ TEST_P(RegexPrecompilationTest, Basic) { absl::StatusOr, const StringValue&, const StringValue&>>:: RegisterGlobalOverload( "prepend", - [](const StringValue& value, const StringValue& prefix) { - return StringValue( - absl::StrCat(prefix.ToString(), value.ToString())); + [](const StringValue& value, const StringValue& prefix, + const Function::InvokeContext& context) { + return StringValue::From( + absl::StrCat(prefix.ToString(), value.ToString()), + context.arena()); }, builder.function_registry()); ASSERT_THAT(status, IsOk()); @@ -114,7 +116,7 @@ TEST_P(RegexPrecompilationTest, Basic) { google::protobuf::Arena arena; Activation activation; activation.InsertOrAssignValue("string_var", - StringValue(&arena, "string_var")); + StringValue::WrapUnsafe("string_var")); ASSERT_OK_AND_ASSIGN(Value value, program->Evaluate(&arena, activation)); EXPECT_THAT(value, test_case.result_matcher); @@ -131,9 +133,11 @@ TEST_P(RegexPrecompilationTest, WithConstantFolding) { absl::StatusOr, const StringValue&, const StringValue&>>:: RegisterGlobalOverload( "prepend", - [](const StringValue& value, const StringValue& prefix) { - return StringValue( - absl::StrCat(prefix.ToString(), value.ToString())); + [](const StringValue& value, const StringValue& prefix, + const Function::InvokeContext& context) { + return StringValue::From( + absl::StrCat(prefix.ToString(), value.ToString()), + context.arena()); }, builder.function_registry()); ASSERT_THAT(status, IsOk()); @@ -158,7 +162,7 @@ TEST_P(RegexPrecompilationTest, WithConstantFolding) { google::protobuf::Arena arena; Activation activation; activation.InsertOrAssignValue("string_var", - StringValue(&arena, "string_var")); + StringValue::WrapUnsafe("string_var")); ASSERT_OK_AND_ASSIGN(Value value, program->Evaluate(&arena, activation)); EXPECT_THAT(value, test_case.result_matcher); diff --git a/runtime/standard/type_conversion_functions.cc b/runtime/standard/type_conversion_functions.cc index 2400c8fdf..09be8d063 100644 --- a/runtime/standard/type_conversion_functions.cc +++ b/runtime/standard/type_conversion_functions.cc @@ -184,7 +184,7 @@ absl::Status RegisterStringConversionFunctions(FunctionRegistry& registry, return ErrorValue( absl::InvalidArgumentError("malformed UTF-8 bytes")); } - return StringValue(value.ToString()); + return StringValue(value); }, registry); CEL_RETURN_IF_ERROR(status); @@ -193,7 +193,7 @@ absl::Status RegisterStringConversionFunctions(FunctionRegistry& registry, status = UnaryFunctionAdapter::RegisterGlobalOverload( cel::builtin::kString, [](bool value) -> StringValue { - return StringValue(value ? "true" : "false"); + return StringValue::WrapUnsafe(value ? "true" : "false"); }, registry); CEL_RETURN_IF_ERROR(status); @@ -209,8 +209,8 @@ absl::Status RegisterStringConversionFunctions(FunctionRegistry& registry, // int -> string status = UnaryFunctionAdapter::RegisterGlobalOverload( cel::builtin::kString, - [](int64_t value) -> StringValue { - return StringValue(absl::StrCat(value)); + [](int64_t value, const Function::InvokeContext& context) -> StringValue { + return StringValue::From(absl::StrCat(value), context.arena()); }, registry); CEL_RETURN_IF_ERROR(status); @@ -225,8 +225,9 @@ absl::Status RegisterStringConversionFunctions(FunctionRegistry& registry, // uint -> string status = UnaryFunctionAdapter::RegisterGlobalOverload( cel::builtin::kString, - [](uint64_t value) -> StringValue { - return StringValue(absl::StrCat(value)); + [](uint64_t value, + const Function::InvokeContext& context) -> StringValue { + return StringValue::From(absl::StrCat(value), context.arena()); }, registry); CEL_RETURN_IF_ERROR(status); @@ -234,12 +235,13 @@ absl::Status RegisterStringConversionFunctions(FunctionRegistry& registry, // duration -> string status = UnaryFunctionAdapter::RegisterGlobalOverload( cel::builtin::kString, - [](absl::Duration value) -> Value { + [](absl::Duration value, + const Function::InvokeContext& context) -> Value { auto encode = EncodeDurationToJson(value); if (!encode.ok()) { return ErrorValue(encode.status()); } - return StringValue(*encode); + return StringValue::From(*encode, context.arena()); }, registry); CEL_RETURN_IF_ERROR(status); @@ -247,12 +249,12 @@ absl::Status RegisterStringConversionFunctions(FunctionRegistry& registry, // timestamp -> string return UnaryFunctionAdapter::RegisterGlobalOverload( cel::builtin::kString, - [](absl::Time value) -> Value { + [](absl::Time value, const Function::InvokeContext& context) -> Value { auto encode = EncodeTimestampToJson(value); if (!encode.ok()) { return ErrorValue(encode.status()); } - return StringValue(*encode); + return StringValue::From(*encode, context.arena()); }, registry); } @@ -320,8 +322,7 @@ absl::Status RegisterBytesConversionFunctions(FunctionRegistry& registry, return UnaryFunctionAdapter, const StringValue&>:: RegisterGlobalOverload( cel::builtin::kBytes, - [](const StringValue& value) { return BytesValue(value.ToString()); }, - registry); + [](const StringValue& value) { return BytesValue(value); }, registry); } absl::Status RegisterDoubleConversionFunctions(FunctionRegistry& registry,