Skip to content

Unwrap compat and parsed legacy list/map wrappers in FromLegacyList and FromLegacyMap, and avoid per-call scratch arenas in legacy map Has. #2363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copybara-service merged 1 commit into from
Oct 13, 2026
Merged

Unwrap compat and parsed legacy list/map wrappers in FromLegacyList and FromLegacyMap, and avoid per-call scratch arenas in legacy map Has. #2363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions common/legacy_value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -1178,10 +1176,9 @@ absl::StatusOr<Value> 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);
Expand Down
39 changes: 39 additions & 0 deletions common/values/legacy_list_value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }

Expand Down Expand Up @@ -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(); }

Expand Down Expand Up @@ -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<LegacyParsedRepeatedFieldListValue>()) {
const auto* impl =
cel::internal::down_cast<const LegacyParsedRepeatedFieldListValue*>(
cel_list);
return CustomListValue(impl, impl->arena());
}
if (native_type_id == NativeTypeId::For<LegacyParsedJsonListValue>()) {
const auto* impl =
cel::internal::down_cast<const LegacyParsedJsonListValue*>(cel_list);
return CustomListValue(impl, impl->arena());
}
if (native_type_id == NativeTypeId::For<CompatListValue>()) {
const auto* impl =
cel::internal::down_cast<const CompatListValue*>(cel_list);
if (google::protobuf::Arena* arena = impl->GetOwningArena(); arena != nullptr) {
return CustomListValue(impl, arena);
}
return CustomListValue();
}
if (native_type_id == NativeTypeId::For<MutableCompatListValue>()) {
const auto* impl =
cel::internal::down_cast<const MutableCompatListValue*>(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,
Expand Down
3 changes: 3 additions & 0 deletions common/values/legacy_list_value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 42 additions & 6 deletions common/values/legacy_map_value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<const google::api::expr::runtime::CelList*> ListKeys()
Expand All @@ -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<LegacyParsedMapFieldMapValue>();
Expand Down Expand Up @@ -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<const google::api::expr::runtime::CelList*> ListKeys()
Expand All @@ -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<LegacyParsedJsonMapValue>();
Expand All @@ -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<LegacyParsedMapFieldMapValue>()) {
const auto* impl =
cel::internal::down_cast<const LegacyParsedMapFieldMapValue*>(
cel_map);
return CustomMapValue(impl, impl->arena());
}
if (native_type_id == NativeTypeId::For<LegacyParsedJsonMapValue>()) {
const auto* impl =
cel::internal::down_cast<const LegacyParsedJsonMapValue*>(cel_map);
return CustomMapValue(impl, impl->arena());
}
if (native_type_id == NativeTypeId::For<CompatMapValue>()) {
const auto* impl =
cel::internal::down_cast<const CompatMapValue*>(cel_map);
if (google::protobuf::Arena* arena = impl->GetOwningArena(); arena != nullptr) {
return CustomMapValue(impl, arena);
}
return CustomMapValue();
}
if (native_type_id == NativeTypeId::For<MutableCompatMapValue>()) {
const auto* impl =
cel::internal::down_cast<const MutableCompatMapValue*>(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,
Expand Down
3 changes: 3 additions & 0 deletions common/values/legacy_map_value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading