Skip to content

Commit ebcffc2

Browse files
authored
Merge pull request #123 from google/cpp-sync
OSS Export
2 parents 3b7f706 + d414f87 commit ebcffc2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1452
-1007
lines changed

conformance/BUILD

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@ cc_binary(
8585
# TODO(issues/114): Ensure the 'in' operator is a logical OR of element equality results.
8686
"--skip_test=comparisons/in_list_literal/elem_in_mixed_type_list_error",
8787
"--skip_test=comparisons/in_map_literal/key_in_mixed_key_type_map_error",
88-
# TODO(issues/115): The 'in' operator fails with maps containing boolean keys.
89-
"--skip_test=fields/in/singleton",
9088
# TODO(issues/97): Parse-only qualified variable lookup "x.y" wtih binding "x.y" or "y" within container "x" fails
9189
"--skip_test=fields/qualified_identifier_resolution/qualified_ident,map_field_select,ident_with_longest_prefix_check,qualified_identifier_resolution_unchecked",
9290
"--skip_test=namespace/qualified/self_eval_qualified_lookup",

eval/compiler/flat_expr_builder_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ TEST(FlatExprBuilderTest, CheckedExprActivationMissesReferences) {
604604
{CelValue::CreateStringView("var2"), CelValue::CreateBool(false)}};
605605

606606
std::unique_ptr<CelMap> map_value =
607-
CreateContainerBackedMap(absl::MakeSpan(map_pairs));
607+
CreateContainerBackedMap(absl::MakeSpan(map_pairs)).value();
608608
activation.InsertValue("bar", CelValue::CreateMap(map_value.get()));
609609
result_or = cel_expr->Evaluate(activation, &arena);
610610
ASSERT_OK(result_or);

eval/eval/BUILD

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ cc_library(
1717
deps = [
1818
":attribute_trail",
1919
":attribute_utility",
20+
":evaluator_stack",
2021
"//base:status_macros",
2122
"//eval/public:activation",
2223
"//eval/public:cel_attribute",
@@ -33,6 +34,31 @@ cc_library(
3334
],
3435
)
3536

37+
cc_library(
38+
name = "evaluator_stack",
39+
srcs = [
40+
"evaluator_stack.cc",
41+
],
42+
hdrs = [
43+
"evaluator_stack.h",
44+
],
45+
deps = [
46+
":attribute_trail",
47+
"//eval/public:cel_value",
48+
],
49+
)
50+
51+
cc_test(
52+
name = "evaluator_stack_test",
53+
srcs = [
54+
"evaluator_stack_test.cc",
55+
],
56+
deps = [
57+
":evaluator_stack",
58+
"@com_google_googletest//:gtest_main",
59+
],
60+
)
61+
3662
cc_library(
3763
name = "expression_step_base",
3864
hdrs = [
@@ -419,6 +445,7 @@ cc_test(
419445
"//eval/public/structs:cel_proto_wrapper",
420446
"//eval/testutil:test_message_cc_proto",
421447
"//testutil:util",
448+
"@com_google_absl//absl/status",
422449
"@com_google_absl//absl/status:statusor",
423450
"@com_google_googleapis//google/api/expr/v1alpha1:syntax_cc_proto",
424451
"@com_google_googletest//:gtest_main",

eval/eval/create_struct_step.cc

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "eval/eval/create_struct_step.h"
22

33
#include <memory>
4+
#include <utility>
45

56
#include "google/api/expr/v1alpha1/syntax.pb.h"
67
#include "absl/status/status.h"
@@ -233,16 +234,17 @@ absl::Status CreateStructStepForMap::DoEvaluate(ExecutionFrame* frame,
233234
map_entries.push_back({args[2 * i], args[2 * i + 1]});
234235
}
235236

236-
auto cel_map =
237+
auto status_or_cel_map =
237238
CreateContainerBackedMap(absl::Span<std::pair<CelValue, CelValue>>(
238239
map_entries.data(), map_entries.size()));
239-
240-
if (cel_map == nullptr) {
241-
*result = CreateErrorValue(frame->arena(), "Failed to create map");
242-
240+
if (!status_or_cel_map.ok()) {
241+
*result =
242+
CreateErrorValue(frame->arena(), status_or_cel_map.status().message());
243243
return absl::OkStatus();
244244
}
245245

246+
auto cel_map = std::move(*status_or_cel_map);
247+
246248
*result = CelValue::CreateMap(cel_map.get());
247249

248250
// Pass object ownership to Arena.

eval/eval/create_struct_step_test.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,8 @@ TEST_P(CreateCreateStructStepTest, TestSetStringMapField) {
593593

594594
auto cel_map =
595595
CreateContainerBackedMap(absl::Span<std::pair<CelValue, CelValue>>(
596-
entries.data(), entries.size()));
596+
entries.data(), entries.size()))
597+
.value();
597598

598599
ASSERT_NO_FATAL_FAILURE(RunExpressionAndGetMessage(
599600
"string_int32_map", CelValue::CreateMap(cel_map.get()), &arena, &test_msg,
@@ -620,7 +621,8 @@ TEST_P(CreateCreateStructStepTest, TestSetInt64MapField) {
620621

621622
auto cel_map =
622623
CreateContainerBackedMap(absl::Span<std::pair<CelValue, CelValue>>(
623-
entries.data(), entries.size()));
624+
entries.data(), entries.size()))
625+
.value();
624626

625627
ASSERT_NO_FATAL_FAILURE(RunExpressionAndGetMessage(
626628
"int64_int32_map", CelValue::CreateMap(cel_map.get()), &arena, &test_msg,
@@ -647,7 +649,8 @@ TEST_P(CreateCreateStructStepTest, TestSetUInt64MapField) {
647649

648650
auto cel_map =
649651
CreateContainerBackedMap(absl::Span<std::pair<CelValue, CelValue>>(
650-
entries.data(), entries.size()));
652+
entries.data(), entries.size()))
653+
.value();
651654

652655
ASSERT_NO_FATAL_FAILURE(RunExpressionAndGetMessage(
653656
"uint64_int32_map", CelValue::CreateMap(cel_map.get()), &arena, &test_msg,

eval/eval/evaluator_core.cc

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,6 @@ absl::Status CheckIterAccess(CelExpressionFlatEvaluationState* state,
3434

3535
} // namespace
3636

37-
void ValueStack::Clear() {
38-
for (auto& v : stack_) {
39-
v = CelValue();
40-
}
41-
for (auto& attr : attribute_stack_) {
42-
attr = AttributeTrail();
43-
}
44-
45-
current_size_ = 0;
46-
}
47-
4837
CelExpressionFlatEvaluationState::CelExpressionFlatEvaluationState(
4938
size_t value_stack_size, const std::set<std::string>& iter_variable_names,
5039
google::protobuf::Arena* arena)
@@ -171,7 +160,7 @@ absl::StatusOr<CelValue> CelExpressionFlatImpl::Trace(
171160
enable_unknowns_, enable_unknown_function_results_,
172161
enable_missing_attribute_errors_);
173162

174-
ValueStack* stack = &frame.value_stack();
163+
EvaluatorStack* stack = &frame.value_stack();
175164
size_t initial_stack_size = stack->size();
176165
const ExpressionStep* expr;
177166
while ((expr = frame.Next()) != nullptr) {

eval/eval/evaluator_core.h

Lines changed: 5 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "absl/types/span.h"
2121
#include "eval/eval/attribute_trail.h"
2222
#include "eval/eval/attribute_utility.h"
23+
#include "eval/eval/evaluator_stack.h"
2324
#include "eval/public/activation.h"
2425
#include "eval/public/cel_attribute.h"
2526
#include "eval/public/cel_expression.h"
@@ -42,7 +43,7 @@ class ExpressionStep {
4243
virtual ~ExpressionStep() {}
4344

4445
// Performs actual evaluation.
45-
// Values are passed between Expression objects via ValueStack, which is
46+
// Values are passed between Expression objects via EvaluatorStack, which is
4647
// supplied with context.
4748
// Also, Expression gets values supplied by caller though Activation
4849
// interface.
@@ -64,122 +65,6 @@ class ExpressionStep {
6465

6566
using ExecutionPath = std::vector<std::unique_ptr<const ExpressionStep>>;
6667

67-
// CelValue stack.
68-
// Implementation is based on vector to allow passing parameters from
69-
// stack as Span<>.
70-
class ValueStack {
71-
public:
72-
explicit ValueStack(size_t max_size) : current_size_(0) {
73-
stack_.resize(max_size);
74-
attribute_stack_.resize(max_size);
75-
}
76-
77-
// Return the current stack size.
78-
size_t size() const { return current_size_; }
79-
80-
// Return the maximum size of the stack.
81-
size_t max_size() const { return stack_.size(); }
82-
83-
// Returns true if stack is empty.
84-
bool empty() const { return current_size_ == 0; }
85-
86-
// Attributes stack size.
87-
size_t attribute_size() const { return current_size_; }
88-
89-
// Check that stack has enough elements.
90-
bool HasEnough(size_t size) const { return current_size_ >= size; }
91-
92-
// Dumps the entire stack state as is.
93-
void Clear();
94-
95-
// Gets the last size elements of the stack.
96-
// Checking that stack has enough elements is caller's responsibility.
97-
// Please note that calls to Push may invalidate returned Span object.
98-
absl::Span<const CelValue> GetSpan(size_t size) const {
99-
if (!HasEnough(size)) {
100-
GOOGLE_LOG(ERROR) << "Requested span size (" << size
101-
<< ") exceeds current stack size: " << current_size_;
102-
}
103-
return absl::Span<const CelValue>(stack_.data() + current_size_ - size,
104-
size);
105-
}
106-
107-
// Gets the last size attribute trails of the stack.
108-
// Checking that stack has enough elements is caller's responsibility.
109-
// Please note that calls to Push may invalidate returned Span object.
110-
absl::Span<const AttributeTrail> GetAttributeSpan(size_t size) const {
111-
return absl::Span<const AttributeTrail>(
112-
attribute_stack_.data() + current_size_ - size, size);
113-
}
114-
115-
// Peeks the last element of the stack.
116-
// Checking that stack is not empty is caller's responsibility.
117-
const CelValue& Peek() const {
118-
if (empty()) {
119-
GOOGLE_LOG(ERROR) << "Peeking on empty ValueStack";
120-
}
121-
return stack_[current_size_ - 1];
122-
}
123-
124-
// Peeks the last element of the attribute stack.
125-
// Checking that stack is not empty is caller's responsibility.
126-
const AttributeTrail& PeekAttribute() const {
127-
if (empty()) {
128-
GOOGLE_LOG(ERROR) << "Peeking on empty ValueStack";
129-
}
130-
return attribute_stack_[current_size_ - 1];
131-
}
132-
133-
// Clears the last size elements of the stack.
134-
// Checking that stack has enough elements is caller's responsibility.
135-
void Pop(size_t size) {
136-
if (!HasEnough(size)) {
137-
GOOGLE_LOG(ERROR) << "Trying to pop more elements (" << size
138-
<< ") than the current stack size: " << current_size_;
139-
}
140-
current_size_ -= size;
141-
}
142-
143-
// Put element on the top of the stack.
144-
void Push(const CelValue& value) { Push(value, AttributeTrail()); }
145-
146-
void Push(const CelValue& value, AttributeTrail attribute) {
147-
if (current_size_ >= stack_.size()) {
148-
GOOGLE_LOG(ERROR) << "No room to push more elements on to ValueStack";
149-
}
150-
stack_[current_size_] = value;
151-
attribute_stack_[current_size_] = attribute;
152-
current_size_++;
153-
}
154-
155-
// Replace element on the top of the stack.
156-
// Checking that stack is not empty is caller's responsibility.
157-
void PopAndPush(const CelValue& value) {
158-
PopAndPush(value, AttributeTrail());
159-
}
160-
161-
// Replace element on the top of the stack.
162-
// Checking that stack is not empty is caller's responsibility.
163-
void PopAndPush(const CelValue& value, AttributeTrail attribute) {
164-
if (empty()) {
165-
GOOGLE_LOG(ERROR) << "Cannot PopAndPush on empty stack.";
166-
}
167-
stack_[current_size_ - 1] = value;
168-
attribute_stack_[current_size_ - 1] = attribute;
169-
}
170-
171-
// Preallocate stack.
172-
void Reserve(size_t size) {
173-
stack_.reserve(size);
174-
attribute_stack_.reserve(size);
175-
}
176-
177-
private:
178-
std::vector<CelValue> stack_;
179-
std::vector<AttributeTrail> attribute_stack_;
180-
size_t current_size_;
181-
};
182-
18368
class CelExpressionFlatEvaluationState : public CelEvaluationState {
18469
public:
18570
CelExpressionFlatEvaluationState(
@@ -196,7 +81,7 @@ class CelExpressionFlatEvaluationState : public CelEvaluationState {
19681

19782
void Reset();
19883

199-
ValueStack& value_stack() { return value_stack_; }
84+
EvaluatorStack& value_stack() { return value_stack_; }
20085

20186
std::vector<IterVarFrame>& iter_stack() { return iter_stack_; }
20287

@@ -207,7 +92,7 @@ class CelExpressionFlatEvaluationState : public CelEvaluationState {
20792
google::protobuf::Arena* arena() { return arena_; }
20893

20994
private:
210-
ValueStack value_stack_;
95+
EvaluatorStack value_stack_;
21196
std::set<std::string> iter_variable_names_;
21297
std::vector<IterVarFrame> iter_stack_;
21398
google::protobuf::Arena* arena_;
@@ -254,7 +139,7 @@ class ExecutionFrame {
254139
return absl::OkStatus();
255140
}
256141

257-
ValueStack& value_stack() { return state_->value_stack(); }
142+
EvaluatorStack& value_stack() { return state_->value_stack(); }
258143
bool enable_unknowns() const { return enable_unknowns_; }
259144
bool enable_unknown_function_results() const {
260145
return enable_unknown_function_results_;

eval/eval/evaluator_core_test.cc

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -53,52 +53,6 @@ class FakeIncrementExpressionStep : public ExpressionStep {
5353
bool ComesFromAst() const override { return true; }
5454
};
5555

56-
// Test Value Stack Push/Pop operation
57-
TEST(EvaluatorCoreTest, ValueStackPushPop) {
58-
google::protobuf::Arena arena;
59-
google::api::expr::v1alpha1::Expr expr;
60-
expr.mutable_ident_expr()->set_name("name");
61-
CelAttribute attribute(expr, {});
62-
ValueStack stack(10);
63-
stack.Push(CelValue::CreateInt64(1));
64-
stack.Push(CelValue::CreateInt64(2), AttributeTrail());
65-
stack.Push(CelValue::CreateInt64(3), AttributeTrail(expr, &arena));
66-
67-
ASSERT_EQ(stack.Peek().Int64OrDie(), 3);
68-
ASSERT_THAT(stack.PeekAttribute().attribute(), NotNull());
69-
ASSERT_EQ(*stack.PeekAttribute().attribute(), attribute);
70-
71-
stack.Pop(1);
72-
73-
ASSERT_EQ(stack.Peek().Int64OrDie(), 2);
74-
ASSERT_EQ(stack.PeekAttribute().attribute(), nullptr);
75-
76-
stack.Pop(1);
77-
78-
ASSERT_EQ(stack.Peek().Int64OrDie(), 1);
79-
ASSERT_EQ(stack.PeekAttribute().attribute(), nullptr);
80-
}
81-
82-
// Test that inner stacks within value stack retain the equality of their sizes.
83-
TEST(EvaluatorCoreTest, ValueStackBalanced) {
84-
ValueStack stack(10);
85-
ASSERT_EQ(stack.size(), stack.attribute_size());
86-
87-
stack.Push(CelValue::CreateInt64(1));
88-
ASSERT_EQ(stack.size(), stack.attribute_size());
89-
stack.Push(CelValue::CreateInt64(2), AttributeTrail());
90-
stack.Push(CelValue::CreateInt64(3), AttributeTrail());
91-
ASSERT_EQ(stack.size(), stack.attribute_size());
92-
93-
stack.PopAndPush(CelValue::CreateInt64(4), AttributeTrail());
94-
ASSERT_EQ(stack.size(), stack.attribute_size());
95-
stack.PopAndPush(CelValue::CreateInt64(5));
96-
ASSERT_EQ(stack.size(), stack.attribute_size());
97-
98-
stack.Pop(3);
99-
ASSERT_EQ(stack.size(), stack.attribute_size());
100-
}
101-
10256
TEST(EvaluatorCoreTest, ExecutionFrameNext) {
10357
ExecutionPath path;
10458
auto const_step = absl::make_unique<const FakeConstExpressionStep>();

eval/eval/evaluator_stack.cc

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#include "eval/eval/evaluator_stack.h"
2+
3+
namespace google {
4+
namespace api {
5+
namespace expr {
6+
namespace runtime {
7+
8+
void EvaluatorStack::Clear() {
9+
for (auto& v : stack_) {
10+
v = CelValue();
11+
}
12+
for (auto& attr : attribute_stack_) {
13+
attr = AttributeTrail();
14+
}
15+
16+
current_size_ = 0;
17+
}
18+
19+
} // namespace runtime
20+
} // namespace expr
21+
} // namespace api
22+
} // namespace google

0 commit comments

Comments
 (0)