Skip to content

Commit 1ca25a6

Browse files
jckingcopybara-github
jcking
authored and
copybara-github
committed
Update comprehensions to use iterators
PiperOrigin-RevId: 743191322
1 parent 76fbb08 commit 1ca25a6

28 files changed

Lines changed: 1606 additions & 855 deletions

‎common/legacy_value.cc‎

Lines changed: 96 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,53 @@ class CelListIterator final : public ValueIterator {
109109
"ValueIterator::Next() called when ValueIterator::HasNext() returns "
110110
"false");
111111
}
112-
auto cel_value = cel_list_->Get(arena, index_++);
112+
auto cel_value = cel_list_->Get(arena, index_);
113113
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_value, *result));
114+
++index_;
114115
return absl::OkStatus();
115116
}
116117

118+
absl::StatusOr<bool> Next1(
119+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
120+
absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
121+
absl::Nonnull<google::protobuf::Arena*> arena,
122+
absl::Nonnull<Value*> key_or_value) override {
123+
ABSL_DCHECK(descriptor_pool != nullptr);
124+
ABSL_DCHECK(message_factory != nullptr);
125+
ABSL_DCHECK(arena != nullptr);
126+
ABSL_DCHECK(key_or_value != nullptr);
127+
128+
if (index_ >= size_) {
129+
return false;
130+
}
131+
auto cel_value = cel_list_->Get(arena, index_);
132+
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_value, *key_or_value));
133+
++index_;
134+
return true;
135+
}
136+
137+
absl::StatusOr<bool> Next2(
138+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
139+
absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
140+
absl::Nonnull<google::protobuf::Arena*> arena, absl::Nonnull<Value*> key,
141+
absl::Nullable<Value*> value) override {
142+
ABSL_DCHECK(descriptor_pool != nullptr);
143+
ABSL_DCHECK(message_factory != nullptr);
144+
ABSL_DCHECK(arena != nullptr);
145+
ABSL_DCHECK(key != nullptr);
146+
147+
if (index_ >= size_) {
148+
return false;
149+
}
150+
if (value != nullptr) {
151+
auto cel_value = cel_list_->Get(arena, index_);
152+
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_value, *value));
153+
}
154+
*key = IntValue(index_);
155+
++index_;
156+
return true;
157+
}
158+
117159
private:
118160
const CelList* const cel_list_;
119161
const int size_;
@@ -137,18 +179,67 @@ class CelMapIterator final : public ValueIterator {
137179
"ValueIterator::Next() called when ValueIterator::HasNext() returns "
138180
"false");
139181
}
140-
ProjectKeys(arena);
141-
CEL_RETURN_IF_ERROR(cel_list_.status());
142-
auto cel_value = (*cel_list_)->Get(arena, index_++);
182+
CEL_RETURN_IF_ERROR(ProjectKeys(arena));
183+
auto cel_value = (*cel_list_)->Get(arena, index_);
143184
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_value, *result));
185+
++index_;
144186
return absl::OkStatus();
145187
}
146188

189+
absl::StatusOr<bool> Next1(
190+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
191+
absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
192+
absl::Nonnull<google::protobuf::Arena*> arena,
193+
absl::Nonnull<Value*> key_or_value) override {
194+
ABSL_DCHECK(descriptor_pool != nullptr);
195+
ABSL_DCHECK(message_factory != nullptr);
196+
ABSL_DCHECK(arena != nullptr);
197+
ABSL_DCHECK(key_or_value != nullptr);
198+
199+
if (index_ >= size_) {
200+
return false;
201+
}
202+
CEL_RETURN_IF_ERROR(ProjectKeys(arena));
203+
auto cel_value = (*cel_list_)->Get(arena, index_);
204+
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_value, *key_or_value));
205+
++index_;
206+
return true;
207+
}
208+
209+
absl::StatusOr<bool> Next2(
210+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
211+
absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
212+
absl::Nonnull<google::protobuf::Arena*> arena, absl::Nonnull<Value*> key,
213+
absl::Nullable<Value*> value) override {
214+
ABSL_DCHECK(descriptor_pool != nullptr);
215+
ABSL_DCHECK(message_factory != nullptr);
216+
ABSL_DCHECK(arena != nullptr);
217+
ABSL_DCHECK(key != nullptr);
218+
219+
if (index_ >= size_) {
220+
return false;
221+
}
222+
CEL_RETURN_IF_ERROR(ProjectKeys(arena));
223+
auto cel_key = (*cel_list_)->Get(arena, index_);
224+
if (value != nullptr) {
225+
auto cel_value = cel_map_->Get(arena, cel_key);
226+
if (!cel_value) {
227+
return absl::DataLossError(
228+
"map iterator returned key that was not present in the map");
229+
}
230+
CEL_RETURN_IF_ERROR(ModernValue(arena, *cel_value, *value));
231+
}
232+
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_key, *key));
233+
++index_;
234+
return true;
235+
}
236+
147237
private:
148-
void ProjectKeys(google::protobuf::Arena* arena) {
238+
absl::Status ProjectKeys(google::protobuf::Arena* arena) {
149239
if (cel_list_.ok() && *cel_list_ == nullptr) {
150240
cel_list_ = cel_map_->ListKeys(arena);
151241
}
242+
return cel_list_.status();
152243
}
153244

154245
const CelMap* const cel_map_;

‎common/value.cc‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
#include "common/values/values.h"
5050
#include "internal/number.h"
5151
#include "internal/protobuf_runtime_version.h"
52+
#include "internal/status_macros.h"
5253
#include "internal/well_known_types.h"
5354
#include "runtime/runtime_options.h"
5455
#include "google/protobuf/arena.h"
@@ -2504,14 +2505,46 @@ class EmptyValueIterator final : public ValueIterator {
25042505
public:
25052506
bool HasNext() override { return false; }
25062507

2507-
absl::Status Next(absl::Nonnull<const google::protobuf::DescriptorPool*>,
2508-
absl::Nonnull<google::protobuf::MessageFactory*>,
2509-
absl::Nonnull<google::protobuf::Arena*>,
2510-
absl::Nonnull<Value*>) override {
2508+
absl::Status Next(
2509+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
2510+
absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
2511+
absl::Nonnull<google::protobuf::Arena*> arena,
2512+
absl::Nonnull<Value*> result) override {
2513+
ABSL_DCHECK(descriptor_pool != nullptr);
2514+
ABSL_DCHECK(message_factory != nullptr);
2515+
ABSL_DCHECK(arena != nullptr);
2516+
ABSL_DCHECK(result != nullptr);
2517+
25112518
return absl::FailedPreconditionError(
25122519
"`ValueIterator::Next` called after `ValueIterator::HasNext` returned "
25132520
"false");
25142521
}
2522+
2523+
absl::StatusOr<bool> Next1(
2524+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
2525+
absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
2526+
absl::Nonnull<google::protobuf::Arena*> arena,
2527+
absl::Nonnull<Value*> key_or_value) override {
2528+
ABSL_DCHECK(descriptor_pool != nullptr);
2529+
ABSL_DCHECK(message_factory != nullptr);
2530+
ABSL_DCHECK(arena != nullptr);
2531+
ABSL_DCHECK(key_or_value != nullptr);
2532+
2533+
return false;
2534+
}
2535+
2536+
absl::StatusOr<bool> Next2(
2537+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
2538+
absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
2539+
absl::Nonnull<google::protobuf::Arena*> arena, absl::Nonnull<Value*> key,
2540+
absl::Nullable<Value*> value) override {
2541+
ABSL_DCHECK(descriptor_pool != nullptr);
2542+
ABSL_DCHECK(message_factory != nullptr);
2543+
ABSL_DCHECK(arena != nullptr);
2544+
ABSL_DCHECK(key != nullptr);
2545+
2546+
return false;
2547+
}
25152548
};
25162549

25172550
} // namespace
@@ -2574,4 +2607,20 @@ bool operator==(DoubleValue lhs, UintValue rhs) {
25742607
internal::Number::FromUint64(rhs.NativeValue());
25752608
}
25762609

2610+
absl::StatusOr<bool> ValueIterator::Next1(
2611+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
2612+
absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
2613+
absl::Nonnull<google::protobuf::Arena*> arena, absl::Nonnull<Value*> value) {
2614+
ABSL_DCHECK(descriptor_pool != nullptr);
2615+
ABSL_DCHECK(message_factory != nullptr);
2616+
ABSL_DCHECK(arena != nullptr);
2617+
ABSL_DCHECK(value != nullptr);
2618+
2619+
if (HasNext()) {
2620+
CEL_RETURN_IF_ERROR(Next(descriptor_pool, message_factory, arena, value));
2621+
return true;
2622+
}
2623+
return false;
2624+
}
2625+
25772626
} // namespace cel

‎common/value.h‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
#include "absl/meta/type_traits.h"
3131
#include "absl/status/status.h"
3232
#include "absl/status/statusor.h"
33-
#include "absl/strings/cord.h"
3433
#include "absl/strings/string_view.h"
3534
#include "absl/types/optional.h"
3635
#include "absl/types/span.h"
@@ -2755,6 +2754,42 @@ inline absl::StatusOr<Value> ValueIterator::Next(
27552754
return result;
27562755
}
27572756

2757+
inline absl::StatusOr<absl::optional<Value>> ValueIterator::Next1(
2758+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
2759+
absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
2760+
absl::Nonnull<google::protobuf::Arena*> arena) {
2761+
ABSL_DCHECK(descriptor_pool != nullptr);
2762+
ABSL_DCHECK(message_factory != nullptr);
2763+
ABSL_DCHECK(arena != nullptr);
2764+
2765+
Value key_or_value;
2766+
CEL_ASSIGN_OR_RETURN(
2767+
bool ok, Next1(descriptor_pool, message_factory, arena, &key_or_value));
2768+
if (!ok) {
2769+
return absl::nullopt;
2770+
}
2771+
return key_or_value;
2772+
}
2773+
2774+
inline absl::StatusOr<absl::optional<std::pair<Value, Value>>>
2775+
ValueIterator::Next2(
2776+
absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
2777+
absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
2778+
absl::Nonnull<google::protobuf::Arena*> arena) {
2779+
ABSL_DCHECK(descriptor_pool != nullptr);
2780+
ABSL_DCHECK(message_factory != nullptr);
2781+
ABSL_DCHECK(arena != nullptr);
2782+
2783+
Value key;
2784+
Value value;
2785+
CEL_ASSIGN_OR_RETURN(
2786+
bool ok, Next2(descriptor_pool, message_factory, arena, &key, &value));
2787+
if (!ok) {
2788+
return absl::nullopt;
2789+
}
2790+
return std::pair{std::move(key), std::move(value)};
2791+
}
2792+
27582793
absl::Nonnull<std::unique_ptr<ValueIterator>> NewEmptyValueIterator();
27592794

27602795
class ValueBuilder {

‎common/value_test.cc‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "absl/base/attributes.h"
2121
#include "absl/log/die_if_null.h"
2222
#include "absl/status/status.h"
23+
#include "absl/status/status_matchers.h"
2324
#include "absl/types/optional.h"
2425
#include "common/type.h"
2526
#include "common/value_testing.h"
@@ -35,6 +36,7 @@
3536
namespace cel {
3637
namespace {
3738

39+
using ::absl_testing::IsOkAndHolds;
3840
using ::absl_testing::StatusIs;
3941
using ::cel::internal::DynamicParseTextProto;
4042
using ::cel::internal::GetTestingDescriptorPool;
@@ -971,5 +973,26 @@ TEST(Value, NumericHeterogeneousEquality) {
971973
EXPECT_NE(DoubleValue(1), UintValue(2));
972974
}
973975

976+
using ValueIteratorTest = common_internal::ValueTest<>;
977+
978+
TEST_F(ValueIteratorTest, Empty) {
979+
auto iterator = NewEmptyValueIterator();
980+
EXPECT_FALSE(iterator->HasNext());
981+
EXPECT_THAT(iterator->Next(descriptor_pool(), message_factory(), arena()),
982+
StatusIs(absl::StatusCode::kFailedPrecondition));
983+
}
984+
985+
TEST_F(ValueIteratorTest, Empty1) {
986+
auto iterator = NewEmptyValueIterator();
987+
EXPECT_THAT(iterator->Next1(descriptor_pool(), message_factory(), arena()),
988+
IsOkAndHolds(Eq(absl::nullopt)));
989+
}
990+
991+
TEST_F(ValueIteratorTest, Empty2) {
992+
auto iterator = NewEmptyValueIterator();
993+
EXPECT_THAT(iterator->Next2(descriptor_pool(), message_factory(), arena()),
994+
IsOkAndHolds(Eq(absl::nullopt)));
995+
}
996+
974997
} // namespace
975998
} // namespace cel

0 commit comments

Comments
 (0)