Skip to content

Commit a1fe92f

Browse files
CEL Dev Teamcopybara-github
authored andcommitted
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: 996898134
1 parent f005075 commit a1fe92f

5 files changed

Lines changed: 91 additions & 13 deletions

File tree

‎common/legacy_value.cc‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,12 +1063,10 @@ absl::Status ModernValue(google::protobuf::Arena* arena,
10631063
result = UnsafeTimestampValue(legacy_value.TimestampOrDie());
10641064
return absl::OkStatus();
10651065
case CelValue::Type::kList:
1066-
result =
1067-
ListValue(common_internal::LegacyListValue(legacy_value.ListOrDie()));
1066+
result = common_internal::FromLegacyList(legacy_value.ListOrDie());
10681067
return absl::OkStatus();
10691068
case CelValue::Type::kMap:
1070-
result =
1071-
MapValue(common_internal::LegacyMapValue(legacy_value.MapOrDie()));
1069+
result = common_internal::FromLegacyMap(legacy_value.MapOrDie());
10721070
return absl::OkStatus();
10731071
case CelValue::Type::kUnknownSet:
10741072
result = common_internal::MakeUnknownValue(
@@ -1178,10 +1176,9 @@ absl::StatusOr<Value> FromLegacyValue(google::protobuf::Arena* arena,
11781176
case CelValue::Type::kTimestamp:
11791177
return UnsafeTimestampValue(legacy_value.TimestampOrDie());
11801178
case CelValue::Type::kList:
1181-
return ListValue(
1182-
common_internal::LegacyListValue(legacy_value.ListOrDie()));
1179+
return common_internal::FromLegacyList(legacy_value.ListOrDie());
11831180
case CelValue::Type::kMap:
1184-
return MapValue(common_internal::LegacyMapValue(legacy_value.MapOrDie()));
1181+
return common_internal::FromLegacyMap(legacy_value.MapOrDie());
11851182
case CelValue::Type::kUnknownSet:
11861183
return common_internal::MakeUnknownValue(*legacy_value.UnknownSetOrDie(),
11871184
arena);

‎common/values/legacy_list_value.cc‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ class LegacyParsedRepeatedFieldListValue final
8080
return UnsafeLegacyValue(result, /*stable=*/false, arena);
8181
}
8282

83+
google::protobuf::Arena* absl_nonnull arena() const { return arena_; }
84+
8385
protected:
8486
std::string DebugString() const override { return value_.DebugString(); }
8587

@@ -202,6 +204,8 @@ class LegacyParsedJsonListValue final
202204
return UnsafeLegacyValue(result, /*stable=*/false, arena);
203205
}
204206

207+
google::protobuf::Arena* absl_nonnull arena() const { return arena_; }
208+
205209
protected:
206210
std::string DebugString() const override { return value_.DebugString(); }
207211

@@ -296,6 +300,41 @@ CustomListValue WrapLegacyParsedJsonList(ParsedJsonListValue value,
296300
arena);
297301
}
298302

303+
ListValue FromLegacyList(
304+
const google::api::expr::runtime::CelList* absl_nullable cel_list) {
305+
if (cel_list != nullptr) {
306+
NativeTypeId native_type_id = NativeTypeId::Of(*cel_list);
307+
if (native_type_id ==
308+
NativeTypeId::For<LegacyParsedRepeatedFieldListValue>()) {
309+
const auto* impl =
310+
cel::internal::down_cast<const LegacyParsedRepeatedFieldListValue*>(
311+
cel_list);
312+
return CustomListValue(impl, impl->arena());
313+
}
314+
if (native_type_id == NativeTypeId::For<LegacyParsedJsonListValue>()) {
315+
const auto* impl =
316+
cel::internal::down_cast<const LegacyParsedJsonListValue*>(cel_list);
317+
return CustomListValue(impl, impl->arena());
318+
}
319+
if (native_type_id == NativeTypeId::For<CompatListValue>()) {
320+
const auto* impl =
321+
cel::internal::down_cast<const CompatListValue*>(cel_list);
322+
if (google::protobuf::Arena* arena = impl->GetOwningArena(); arena != nullptr) {
323+
return CustomListValue(impl, arena);
324+
}
325+
return CustomListValue();
326+
}
327+
if (native_type_id == NativeTypeId::For<MutableCompatListValue>()) {
328+
const auto* impl =
329+
cel::internal::down_cast<const MutableCompatListValue*>(cel_list);
330+
if (google::protobuf::Arena* arena = impl->GetOwningArena(); arena != nullptr) {
331+
return CustomListValue(impl, arena);
332+
}
333+
}
334+
}
335+
return LegacyListValue(cel_list);
336+
}
337+
299338
absl::Status LegacyListValue::Equal(
300339
const Value& other,
301340
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,

‎common/values/legacy_list_value.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,9 @@ CustomListValue WrapLegacyParsedRepeatedField(
171171
CustomListValue WrapLegacyParsedJsonList(ParsedJsonListValue value,
172172
google::protobuf::Arena* absl_nonnull arena);
173173

174+
ListValue FromLegacyList(
175+
const google::api::expr::runtime::CelList* absl_nullable cel_list);
176+
174177
} // namespace common_internal
175178

176179
} // namespace cel

‎common/values/legacy_map_value.cc‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,10 @@ class LegacyParsedMapFieldMapValue final
253253
const google::api::expr::runtime::CelValue& key) const override {
254254
CEL_RETURN_IF_ERROR(
255255
google::api::expr::runtime::CelValue::CheckMapKeyType(key));
256-
google::protobuf::Arena scratch_arena;
257256
Value modern_key;
258-
CEL_RETURN_IF_ERROR(ModernValue(&scratch_arena, key, modern_key));
257+
CEL_RETURN_IF_ERROR(ModernValue(arena_, key, modern_key));
259258
return Has(modern_key, google::protobuf::DescriptorPool::generated_pool(),
260-
google::protobuf::MessageFactory::generated_factory(), &scratch_arena);
259+
google::protobuf::MessageFactory::generated_factory(), arena_);
261260
}
262261

263262
absl::StatusOr<const google::api::expr::runtime::CelList*> ListKeys()
@@ -281,6 +280,8 @@ class LegacyParsedMapFieldMapValue final
281280
return legacy_list->cel_list();
282281
}
283282

283+
google::protobuf::Arena* absl_nonnull arena() const { return arena_; }
284+
284285
private:
285286
NativeTypeId GetNativeTypeId() const override {
286287
return NativeTypeId::For<LegacyParsedMapFieldMapValue>();
@@ -440,11 +441,10 @@ class LegacyParsedJsonMapValue final
440441
const google::api::expr::runtime::CelValue& key) const override {
441442
CEL_RETURN_IF_ERROR(
442443
google::api::expr::runtime::CelValue::CheckMapKeyType(key));
443-
google::protobuf::Arena scratch_arena;
444444
Value modern_key;
445-
CEL_RETURN_IF_ERROR(ModernValue(&scratch_arena, key, modern_key));
445+
CEL_RETURN_IF_ERROR(ModernValue(arena_, key, modern_key));
446446
return Has(modern_key, google::protobuf::DescriptorPool::generated_pool(),
447-
google::protobuf::MessageFactory::generated_factory(), &scratch_arena);
447+
google::protobuf::MessageFactory::generated_factory(), arena_);
448448
}
449449

450450
absl::StatusOr<const google::api::expr::runtime::CelList*> ListKeys()
@@ -468,6 +468,8 @@ class LegacyParsedJsonMapValue final
468468
return legacy_list->cel_list();
469469
}
470470

471+
google::protobuf::Arena* absl_nonnull arena() const { return arena_; }
472+
471473
private:
472474
NativeTypeId GetNativeTypeId() const override {
473475
return NativeTypeId::For<LegacyParsedJsonMapValue>();
@@ -491,6 +493,40 @@ CustomMapValue WrapLegacyParsedJsonMap(ParsedJsonMapValue value,
491493
arena);
492494
}
493495

496+
MapValue FromLegacyMap(
497+
const google::api::expr::runtime::CelMap* absl_nullable cel_map) {
498+
if (cel_map != nullptr) {
499+
NativeTypeId native_type_id = NativeTypeId::Of(*cel_map);
500+
if (native_type_id == NativeTypeId::For<LegacyParsedMapFieldMapValue>()) {
501+
const auto* impl =
502+
cel::internal::down_cast<const LegacyParsedMapFieldMapValue*>(
503+
cel_map);
504+
return CustomMapValue(impl, impl->arena());
505+
}
506+
if (native_type_id == NativeTypeId::For<LegacyParsedJsonMapValue>()) {
507+
const auto* impl =
508+
cel::internal::down_cast<const LegacyParsedJsonMapValue*>(cel_map);
509+
return CustomMapValue(impl, impl->arena());
510+
}
511+
if (native_type_id == NativeTypeId::For<CompatMapValue>()) {
512+
const auto* impl =
513+
cel::internal::down_cast<const CompatMapValue*>(cel_map);
514+
if (google::protobuf::Arena* arena = impl->GetOwningArena(); arena != nullptr) {
515+
return CustomMapValue(impl, arena);
516+
}
517+
return CustomMapValue();
518+
}
519+
if (native_type_id == NativeTypeId::For<MutableCompatMapValue>()) {
520+
const auto* impl =
521+
cel::internal::down_cast<const MutableCompatMapValue*>(cel_map);
522+
if (google::protobuf::Arena* arena = impl->GetOwningArena(); arena != nullptr) {
523+
return CustomMapValue(impl, arena);
524+
}
525+
}
526+
}
527+
return LegacyMapValue(cel_map);
528+
}
529+
494530
absl::Status LegacyMapValue::Equal(
495531
const Value& other,
496532
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,

‎common/values/legacy_map_value.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,9 @@ CustomMapValue WrapLegacyParsedMapField(ParsedMapFieldValue value,
188188
CustomMapValue WrapLegacyParsedJsonMap(ParsedJsonMapValue value,
189189
google::protobuf::Arena* absl_nonnull arena);
190190

191+
MapValue FromLegacyMap(
192+
const google::api::expr::runtime::CelMap* absl_nullable cel_map);
193+
191194
} // namespace common_internal
192195

193196
} // namespace cel

0 commit comments

Comments
 (0)