Skip to content

Commit f6396c1

Browse files
jckingcopybara-github
jcking
authored and
copybara-github
committed
Remove ValueManager& from NewIterator()
PiperOrigin-RevId: 723576825
1 parent 148c393 commit f6396c1

37 files changed

+124
-125
lines changed

‎common/legacy_value.cc‎

Lines changed: 50 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -114,29 +114,63 @@ MessageWrapper AsMessageWrapper(uintptr_t message_ptr, uintptr_t type_info) {
114114

115115
class CelListIterator final : public ValueIterator {
116116
public:
117-
CelListIterator(google::protobuf::Arena* arena, const CelList* cel_list)
118-
: arena_(arena), cel_list_(cel_list), size_(cel_list_->size()) {}
117+
explicit CelListIterator(const CelList* cel_list)
118+
: cel_list_(cel_list), size_(cel_list_->size()) {}
119119

120120
bool HasNext() override { return index_ < size_; }
121121

122-
absl::Status Next(ValueManager&, Value& result) override {
122+
absl::Status Next(ValueManager& value_manager, Value& result) override {
123123
if (!HasNext()) {
124124
return absl::FailedPreconditionError(
125125
"ValueIterator::Next() called when ValueIterator::HasNext() returns "
126126
"false");
127127
}
128-
auto cel_value = cel_list_->Get(arena_, index_++);
129-
CEL_RETURN_IF_ERROR(ModernValue(arena_, cel_value, result));
128+
auto* arena = value_manager.GetMemoryManager().arena();
129+
auto cel_value = cel_list_->Get(arena, index_++);
130+
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_value, result));
130131
return absl::OkStatus();
131132
}
132133

133134
private:
134-
google::protobuf::Arena* const arena_;
135135
const CelList* const cel_list_;
136136
const int size_;
137137
int index_ = 0;
138138
};
139139

140+
class CelMapIterator final : public ValueIterator {
141+
public:
142+
explicit CelMapIterator(const CelMap* cel_map)
143+
: cel_map_(cel_map), size_(cel_map->size()) {}
144+
145+
bool HasNext() override { return index_ < size_; }
146+
147+
absl::Status Next(ValueManager& value_manager, Value& result) override {
148+
if (!HasNext()) {
149+
return absl::FailedPreconditionError(
150+
"ValueIterator::Next() called when ValueIterator::HasNext() returns "
151+
"false");
152+
}
153+
ProjectKeys(value_manager.GetMemoryManager().arena());
154+
CEL_RETURN_IF_ERROR(cel_list_.status());
155+
auto* arena = value_manager.GetMemoryManager().arena();
156+
auto cel_value = (*cel_list_)->Get(arena, index_++);
157+
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_value, result));
158+
return absl::OkStatus();
159+
}
160+
161+
private:
162+
void ProjectKeys(google::protobuf::Arena* arena) {
163+
if (cel_list_.ok() && *cel_list_ == nullptr) {
164+
cel_list_ = cel_map_->ListKeys(arena);
165+
}
166+
}
167+
168+
const CelMap* const cel_map_;
169+
const int size_ = 0;
170+
absl::StatusOr<const CelList*> cel_list_ = nullptr;
171+
int index_ = 0;
172+
};
173+
140174
std::string cel_common_internal_LegacyListValue_DebugString(uintptr_t impl) {
141175
return CelValue::CreateList(AsCelList(impl)).DebugString();
142176
}
@@ -182,11 +216,8 @@ absl::Status cel_common_internal_LegacyListValue_ForEach(
182216
}
183217

184218
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>>
185-
cel_common_internal_LegacyListValue_NewIterator(uintptr_t impl,
186-
ValueManager& value_manager) {
187-
return std::make_unique<CelListIterator>(
188-
extensions::ProtoMemoryManagerArena(value_manager.GetMemoryManager()),
189-
AsCelList(impl));
219+
cel_common_internal_LegacyListValue_NewIterator(uintptr_t impl) {
220+
return std::make_unique<CelListIterator>(AsCelList(impl));
190221
}
191222

192223
absl::Status cel_common_internal_LegacyListValue_Contains(
@@ -489,9 +520,9 @@ absl::Status LegacyListValue::ForEach(ValueManager& value_manager,
489520
callback);
490521
}
491522

492-
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> LegacyListValue::NewIterator(
493-
ValueManager& value_manager) const {
494-
return cel_common_internal_LegacyListValue_NewIterator(impl_, value_manager);
523+
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> LegacyListValue::NewIterator()
524+
const {
525+
return cel_common_internal_LegacyListValue_NewIterator(impl_);
495526
}
496527

497528
absl::Status LegacyListValue::Contains(ValueManager& value_manager,
@@ -644,13 +675,8 @@ absl::Status cel_common_internal_LegacyMapValue_ForEach(
644675
}
645676

646677
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>>
647-
cel_common_internal_LegacyMapValue_NewIterator(uintptr_t impl,
648-
ValueManager& value_manager) {
649-
auto* arena =
650-
extensions::ProtoMemoryManagerArena(value_manager.GetMemoryManager());
651-
CEL_ASSIGN_OR_RETURN(auto keys, AsCelMap(impl)->ListKeys(arena));
652-
return cel_common_internal_LegacyListValue_NewIterator(
653-
reinterpret_cast<uintptr_t>(keys), value_manager);
678+
cel_common_internal_LegacyMapValue_NewIterator(uintptr_t impl) {
679+
return std::make_unique<CelMapIterator>(AsCelMap(impl));
654680
}
655681

656682
} // namespace
@@ -800,9 +826,9 @@ absl::Status LegacyMapValue::ForEach(ValueManager& value_manager,
800826
callback);
801827
}
802828

803-
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> LegacyMapValue::NewIterator(
804-
ValueManager& value_manager) const {
805-
return cel_common_internal_LegacyMapValue_NewIterator(impl_, value_manager);
829+
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> LegacyMapValue::NewIterator()
830+
const {
831+
return cel_common_internal_LegacyMapValue_NewIterator(impl_);
806832
}
807833

808834
} // namespace common_internal

‎common/value.cc‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -497,12 +497,11 @@ absl::Status ListValue::ForEach(ValueManager& value_manager,
497497
variant_);
498498
}
499499

500-
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> ListValue::NewIterator(
501-
ValueManager& value_manager) const {
500+
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> ListValue::NewIterator() const {
502501
return absl::visit(
503-
[&value_manager](const auto& alternative)
502+
[](const auto& alternative)
504503
-> absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> {
505-
return alternative.NewIterator(value_manager);
504+
return alternative.NewIterator();
506505
},
507506
variant_);
508507
}
@@ -608,12 +607,11 @@ absl::Status MapValue::ForEach(ValueManager& value_manager,
608607
variant_);
609608
}
610609

611-
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> MapValue::NewIterator(
612-
ValueManager& value_manager) const {
610+
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> MapValue::NewIterator() const {
613611
return absl::visit(
614-
[&value_manager](const auto& alternative)
612+
[](const auto& alternative)
615613
-> absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> {
616-
return alternative.NewIterator(value_manager);
614+
return alternative.NewIterator();
617615
},
618616
variant_);
619617
}

‎common/value.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2719,8 +2719,8 @@ inline absl::Status CustomListValue::ForEach(
27192719
}
27202720

27212721
inline absl::StatusOr<absl::Nonnull<ValueIteratorPtr>>
2722-
CustomListValue::NewIterator(ValueManager& value_manager) const {
2723-
return interface_->NewIterator(value_manager);
2722+
CustomListValue::NewIterator() const {
2723+
return interface_->NewIterator();
27242724
}
27252725

27262726
inline absl::Status CustomListValue::Equal(ValueManager& value_manager,
@@ -2780,8 +2780,8 @@ inline absl::Status CustomMapValue::ForEach(ValueManager& value_manager,
27802780
}
27812781

27822782
inline absl::StatusOr<absl::Nonnull<ValueIteratorPtr>>
2783-
CustomMapValue::NewIterator(ValueManager& value_manager) const {
2784-
return interface_->NewIterator(value_manager);
2783+
CustomMapValue::NewIterator() const {
2784+
return interface_->NewIterator();
27852785
}
27862786

27872787
inline absl::Status CustomMapValue::Equal(ValueManager& value_manager,

‎common/values/custom_list_value.cc‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,25 +131,22 @@ absl::Nonnull<const CompatListValue*> EmptyCompatListValue() {
131131
class CustomListValueInterfaceIterator final : public ValueIterator {
132132
public:
133133
explicit CustomListValueInterfaceIterator(
134-
const CustomListValueInterface& interface, ValueManager& value_manager)
135-
: interface_(interface),
136-
value_manager_(value_manager),
137-
size_(interface_.Size()) {}
134+
const CustomListValueInterface& interface)
135+
: interface_(interface), size_(interface_.Size()) {}
138136

139137
bool HasNext() override { return index_ < size_; }
140138

141-
absl::Status Next(ValueManager&, Value& result) override {
139+
absl::Status Next(ValueManager& value_manager, Value& result) override {
142140
if (ABSL_PREDICT_FALSE(index_ >= size_)) {
143141
return absl::FailedPreconditionError(
144142
"ValueIterator::Next() called when "
145143
"ValueIterator::HasNext() returns false");
146144
}
147-
return interface_.GetImpl(value_manager_, index_++, result);
145+
return interface_.GetImpl(value_manager, index_++, result);
148146
}
149147

150148
private:
151149
const CustomListValueInterface& interface_;
152-
ValueManager& value_manager_;
153150
const size_t size_;
154151
size_t index_ = 0;
155152
};
@@ -214,9 +211,8 @@ absl::Status CustomListValueInterface::ForEach(
214211
}
215212

216213
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>>
217-
CustomListValueInterface::NewIterator(ValueManager& value_manager) const {
218-
return std::make_unique<CustomListValueInterfaceIterator>(*this,
219-
value_manager);
214+
CustomListValueInterface::NewIterator() const {
215+
return std::make_unique<CustomListValueInterfaceIterator>(*this);
220216
}
221217

222218
absl::Status CustomListValueInterface::Equal(ValueManager& value_manager,

‎common/values/custom_list_value.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,7 @@ class CustomListValueInterface : public CustomValueInterface {
9797
virtual absl::Status ForEach(ValueManager& value_manager,
9898
ForEachWithIndexCallback callback) const;
9999

100-
virtual absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator(
101-
ValueManager& value_manager) const;
100+
virtual absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator() const;
102101

103102
virtual absl::Status Contains(ValueManager& value_manager, const Value& other,
104103
Value& result) const;
@@ -187,8 +186,7 @@ class CustomListValue {
187186
absl::Status ForEach(ValueManager& value_manager,
188187
ForEachWithIndexCallback callback) const;
189188

190-
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator(
191-
ValueManager& value_manager) const;
189+
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator() const;
192190

193191
absl::Status Contains(ValueManager& value_manager, const Value& other,
194192
Value& result) const;

‎common/values/custom_map_value.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,7 @@ class EmptyMapValue final : public common_internal::CompatMapValue {
9191
return absl::OkStatus();
9292
}
9393

94-
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator(
95-
ValueManager&) const override {
94+
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator() const override {
9695
return std::make_unique<EmptyMapValueKeyIterator>();
9796
}
9897

@@ -273,7 +272,7 @@ absl::Status CustomMapValueInterface::Has(ValueManager& value_manager,
273272

274273
absl::Status CustomMapValueInterface::ForEach(ValueManager& value_manager,
275274
ForEachCallback callback) const {
276-
CEL_ASSIGN_OR_RETURN(auto iterator, NewIterator(value_manager));
275+
CEL_ASSIGN_OR_RETURN(auto iterator, NewIterator());
277276
while (iterator->HasNext()) {
278277
Value key;
279278
Value value;

‎common/values/custom_map_value.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,8 @@ class CustomMapValueInterface : public CustomValueInterface {
111111

112112
// By default, implementations do not guarantee any iteration order. Unless
113113
// specified otherwise, assume the iteration order is random.
114-
virtual absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator(
115-
ValueManager& value_manager) const = 0;
114+
virtual absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator()
115+
const = 0;
116116

117117
virtual CustomMapValue Clone(ArenaAllocator<> allocator) const = 0;
118118

@@ -217,8 +217,7 @@ class CustomMapValue {
217217

218218
// See the corresponding member function of `MapValueInterface` for
219219
// documentation.
220-
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator(
221-
ValueManager& value_manager) const;
220+
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator() const;
222221

223222
void swap(CustomMapValue& other) noexcept {
224223
using std::swap;

‎common/values/legacy_list_value.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ class LegacyListValue final {
111111
absl::Status ForEach(ValueManager& value_manager,
112112
ForEachWithIndexCallback callback) const;
113113

114-
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator(
115-
ValueManager& value_manager) const;
114+
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator() const;
116115

117116
void swap(LegacyListValue& other) noexcept {
118117
using std::swap;

‎common/values/legacy_map_value.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,7 @@ class LegacyMapValue final {
112112
absl::Status ForEach(ValueManager& value_manager,
113113
ForEachCallback callback) const;
114114

115-
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator(
116-
ValueManager& value_manager) const;
115+
absl::StatusOr<absl::Nonnull<ValueIteratorPtr>> NewIterator() const;
117116

118117
void swap(LegacyMapValue& other) noexcept {
119118
using std::swap;

‎common/values/list_value.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ absl::Status ListValueEqual(ValueManager& value_manager, const ListValue& lhs,
115115
result = BoolValue{false};
116116
return absl::OkStatus();
117117
}
118-
CEL_ASSIGN_OR_RETURN(auto lhs_iterator, lhs.NewIterator(value_manager));
119-
CEL_ASSIGN_OR_RETURN(auto rhs_iterator, rhs.NewIterator(value_manager));
118+
CEL_ASSIGN_OR_RETURN(auto lhs_iterator, lhs.NewIterator());
119+
CEL_ASSIGN_OR_RETURN(auto rhs_iterator, rhs.NewIterator());
120120
Value lhs_element;
121121
Value rhs_element;
122122
for (size_t index = 0; index < lhs_size; ++index) {
@@ -145,8 +145,8 @@ absl::Status ListValueEqual(ValueManager& value_manager,
145145
result = BoolValue{false};
146146
return absl::OkStatus();
147147
}
148-
CEL_ASSIGN_OR_RETURN(auto lhs_iterator, lhs.NewIterator(value_manager));
149-
CEL_ASSIGN_OR_RETURN(auto rhs_iterator, rhs.NewIterator(value_manager));
148+
CEL_ASSIGN_OR_RETURN(auto lhs_iterator, lhs.NewIterator());
149+
CEL_ASSIGN_OR_RETURN(auto rhs_iterator, rhs.NewIterator());
150150
Value lhs_element;
151151
Value rhs_element;
152152
for (size_t index = 0; index < lhs_size; ++index) {

0 commit comments

Comments
 (0)