From 21a5734c805b086a5f4241471c9201bf45a7bea2 Mon Sep 17 00:00:00 2001 From: CEL Dev Team Date: Tue, 13 Oct 2026 08:41:57 -0700 Subject: [PATCH] Unwrap compat and parsed legacy list/map wrappers in `FromLegacyList` and `FromLegacyMap`, and avoid per-call scratch arenas in legacy map `Has`. 1. Add `FromLegacyList` and `FromLegacyMap` (used by `ModernValue` and `FromLegacyValue`) to unwrap `LegacyParsedRepeatedFieldListValue`, `LegacyParsedJsonListValue`, `CompatListValue`, and `MutableCompatListValue` (and their map counterparts `LegacyParsedMapFieldMapValue`, `LegacyParsedJsonMapValue`, `CompatMapValue`, and `MutableCompatMapValue`) back into `CustomListValue` / `CustomMapValue` using their owning arena instead of re-wrapping them in `LegacyListValue` / `LegacyMapValue`. 2. In `LegacyParsedMapFieldMapValue::Has(const CelValue&)` and `LegacyParsedJsonMapValue::Has(const CelValue&)`, use `arena_` instead of constructing a per-call `proto2::Arena scratch_arena`. PiperOrigin-RevId: 998661512 --- common/legacy_value.cc | 11 +++---- common/values/legacy_list_value.cc | 39 ++++++++++++++++++++++++ common/values/legacy_list_value.h | 3 ++ common/values/legacy_map_value.cc | 48 ++++++++++++++++++++++++++---- common/values/legacy_map_value.h | 3 ++ 5 files changed, 91 insertions(+), 13 deletions(-) diff --git a/common/legacy_value.cc b/common/legacy_value.cc index 622257f43..151d848af 100644 --- a/common/legacy_value.cc +++ b/common/legacy_value.cc @@ -1063,12 +1063,10 @@ absl::Status ModernValue(google::protobuf::Arena* arena, result = UnsafeTimestampValue(legacy_value.TimestampOrDie()); return absl::OkStatus(); case CelValue::Type::kList: - result = - ListValue(common_internal::LegacyListValue(legacy_value.ListOrDie())); + result = common_internal::FromLegacyList(legacy_value.ListOrDie()); return absl::OkStatus(); case CelValue::Type::kMap: - result = - MapValue(common_internal::LegacyMapValue(legacy_value.MapOrDie())); + result = common_internal::FromLegacyMap(legacy_value.MapOrDie()); return absl::OkStatus(); case CelValue::Type::kUnknownSet: result = common_internal::MakeUnknownValue( @@ -1178,10 +1176,9 @@ absl::StatusOr FromLegacyValue(google::protobuf::Arena* arena, case CelValue::Type::kTimestamp: return UnsafeTimestampValue(legacy_value.TimestampOrDie()); case CelValue::Type::kList: - return ListValue( - common_internal::LegacyListValue(legacy_value.ListOrDie())); + return common_internal::FromLegacyList(legacy_value.ListOrDie()); case CelValue::Type::kMap: - return MapValue(common_internal::LegacyMapValue(legacy_value.MapOrDie())); + return common_internal::FromLegacyMap(legacy_value.MapOrDie()); case CelValue::Type::kUnknownSet: return common_internal::MakeUnknownValue(*legacy_value.UnknownSetOrDie(), arena); diff --git a/common/values/legacy_list_value.cc b/common/values/legacy_list_value.cc index d3c1b94be..50bf1e6fe 100644 --- a/common/values/legacy_list_value.cc +++ b/common/values/legacy_list_value.cc @@ -80,6 +80,8 @@ class LegacyParsedRepeatedFieldListValue final return UnsafeLegacyValue(result, /*stable=*/false, arena); } + google::protobuf::Arena* absl_nonnull arena() const { return arena_; } + protected: std::string DebugString() const override { return value_.DebugString(); } @@ -202,6 +204,8 @@ class LegacyParsedJsonListValue final return UnsafeLegacyValue(result, /*stable=*/false, arena); } + google::protobuf::Arena* absl_nonnull arena() const { return arena_; } + protected: std::string DebugString() const override { return value_.DebugString(); } @@ -296,6 +300,41 @@ CustomListValue WrapLegacyParsedJsonList(ParsedJsonListValue value, arena); } +ListValue FromLegacyList( + const google::api::expr::runtime::CelList* absl_nullable cel_list) { + if (cel_list != nullptr) { + NativeTypeId native_type_id = NativeTypeId::Of(*cel_list); + if (native_type_id == + NativeTypeId::For()) { + const auto* impl = + cel::internal::down_cast( + cel_list); + return CustomListValue(impl, impl->arena()); + } + if (native_type_id == NativeTypeId::For()) { + const auto* impl = + cel::internal::down_cast(cel_list); + return CustomListValue(impl, impl->arena()); + } + if (native_type_id == NativeTypeId::For()) { + const auto* impl = + cel::internal::down_cast(cel_list); + if (google::protobuf::Arena* arena = impl->GetOwningArena(); arena != nullptr) { + return CustomListValue(impl, arena); + } + return CustomListValue(); + } + if (native_type_id == NativeTypeId::For()) { + const auto* impl = + cel::internal::down_cast(cel_list); + if (google::protobuf::Arena* arena = impl->GetOwningArena(); arena != nullptr) { + return CustomListValue(impl, arena); + } + } + } + return LegacyListValue(cel_list); +} + absl::Status LegacyListValue::Equal( const Value& other, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, diff --git a/common/values/legacy_list_value.h b/common/values/legacy_list_value.h index 6dd6a568e..55a169653 100644 --- a/common/values/legacy_list_value.h +++ b/common/values/legacy_list_value.h @@ -171,6 +171,9 @@ CustomListValue WrapLegacyParsedRepeatedField( CustomListValue WrapLegacyParsedJsonList(ParsedJsonListValue value, google::protobuf::Arena* absl_nonnull arena); +ListValue FromLegacyList( + const google::api::expr::runtime::CelList* absl_nullable cel_list); + } // namespace common_internal } // namespace cel diff --git a/common/values/legacy_map_value.cc b/common/values/legacy_map_value.cc index 8d9ce2ad3..bd025c51b 100644 --- a/common/values/legacy_map_value.cc +++ b/common/values/legacy_map_value.cc @@ -253,11 +253,10 @@ class LegacyParsedMapFieldMapValue final const google::api::expr::runtime::CelValue& key) const override { CEL_RETURN_IF_ERROR( google::api::expr::runtime::CelValue::CheckMapKeyType(key)); - google::protobuf::Arena scratch_arena; Value modern_key; - CEL_RETURN_IF_ERROR(ModernValue(&scratch_arena, key, modern_key)); + CEL_RETURN_IF_ERROR(ModernValue(arena_, key, modern_key)); return Has(modern_key, google::protobuf::DescriptorPool::generated_pool(), - google::protobuf::MessageFactory::generated_factory(), &scratch_arena); + google::protobuf::MessageFactory::generated_factory(), arena_); } absl::StatusOr ListKeys() @@ -281,6 +280,8 @@ class LegacyParsedMapFieldMapValue final return legacy_list->cel_list(); } + google::protobuf::Arena* absl_nonnull arena() const { return arena_; } + private: NativeTypeId GetNativeTypeId() const override { return NativeTypeId::For(); @@ -440,11 +441,10 @@ class LegacyParsedJsonMapValue final const google::api::expr::runtime::CelValue& key) const override { CEL_RETURN_IF_ERROR( google::api::expr::runtime::CelValue::CheckMapKeyType(key)); - google::protobuf::Arena scratch_arena; Value modern_key; - CEL_RETURN_IF_ERROR(ModernValue(&scratch_arena, key, modern_key)); + CEL_RETURN_IF_ERROR(ModernValue(arena_, key, modern_key)); return Has(modern_key, google::protobuf::DescriptorPool::generated_pool(), - google::protobuf::MessageFactory::generated_factory(), &scratch_arena); + google::protobuf::MessageFactory::generated_factory(), arena_); } absl::StatusOr ListKeys() @@ -468,6 +468,8 @@ class LegacyParsedJsonMapValue final return legacy_list->cel_list(); } + google::protobuf::Arena* absl_nonnull arena() const { return arena_; } + private: NativeTypeId GetNativeTypeId() const override { return NativeTypeId::For(); @@ -491,6 +493,40 @@ CustomMapValue WrapLegacyParsedJsonMap(ParsedJsonMapValue value, arena); } +MapValue FromLegacyMap( + const google::api::expr::runtime::CelMap* absl_nullable cel_map) { + if (cel_map != nullptr) { + NativeTypeId native_type_id = NativeTypeId::Of(*cel_map); + if (native_type_id == NativeTypeId::For()) { + const auto* impl = + cel::internal::down_cast( + cel_map); + return CustomMapValue(impl, impl->arena()); + } + if (native_type_id == NativeTypeId::For()) { + const auto* impl = + cel::internal::down_cast(cel_map); + return CustomMapValue(impl, impl->arena()); + } + if (native_type_id == NativeTypeId::For()) { + const auto* impl = + cel::internal::down_cast(cel_map); + if (google::protobuf::Arena* arena = impl->GetOwningArena(); arena != nullptr) { + return CustomMapValue(impl, arena); + } + return CustomMapValue(); + } + if (native_type_id == NativeTypeId::For()) { + const auto* impl = + cel::internal::down_cast(cel_map); + if (google::protobuf::Arena* arena = impl->GetOwningArena(); arena != nullptr) { + return CustomMapValue(impl, arena); + } + } + } + return LegacyMapValue(cel_map); +} + absl::Status LegacyMapValue::Equal( const Value& other, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, diff --git a/common/values/legacy_map_value.h b/common/values/legacy_map_value.h index 53f39a8de..c170dea80 100644 --- a/common/values/legacy_map_value.h +++ b/common/values/legacy_map_value.h @@ -188,6 +188,9 @@ CustomMapValue WrapLegacyParsedMapField(ParsedMapFieldValue value, CustomMapValue WrapLegacyParsedJsonMap(ParsedJsonMapValue value, google::protobuf::Arena* absl_nonnull arena); +MapValue FromLegacyMap( + const google::api::expr::runtime::CelMap* absl_nullable cel_map); + } // namespace common_internal } // namespace cel