Skip to content

Commit

Permalink
Merge pull request #58 from google/cpp-sync
Browse files Browse the repository at this point in the history
Export of internal changes
  • Loading branch information
TristonianJones authored May 11, 2020
2 parents d88a482 + 3506ace commit b416ef7
Show file tree
Hide file tree
Showing 23 changed files with 262 additions and 123 deletions.
6 changes: 3 additions & 3 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,9 @@ antlr_dependencies(472)

http_archive(
name = "antlr4_runtimes",
sha256 = "4d0714f441333a63e50031c9e8e4890c78f3d21e053d46416949803e122a6574",
strip_prefix = "antlr4-4.7.1",
urls = ["https://github.com/antlr/antlr4/archive/4.7.1.tar.gz"],
sha256 = "46f5e1af5f4bd28ade55cb632f9a069656b31fc8c2408f9aa045f9b5f5caad64",
strip_prefix = "antlr4-4.7.2",
urls = ["https://github.com/antlr/antlr4/archive/4.7.2.tar.gz"],
build_file_content = """
package(default_visibility = ["//visibility:public"])
cc_library(
Expand Down
1 change: 1 addition & 0 deletions bazel/antlr.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,5 @@ def antlr_cc_library(name, src, listener = False, visitor = True):
toolchains = [":" + generated],
# ANTLR runtime does not build with dynamic linking
linkstatic = True,
alwayslink = 1,
)
2 changes: 1 addition & 1 deletion eval/compiler/flat_expr_builder_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ TEST(FlatExprBuilderTest, DelayedFunctionResolutionErrors) {

ASSERT_THAT(warnings, testing::SizeIs(1));
EXPECT_EQ(warnings[0].code(), absl::StatusCode::kInvalidArgument);
EXPECT_THAT(warnings[0].message(),
EXPECT_THAT(std::string(warnings[0].message()),
testing::HasSubstr("No overloads provided"));
}

Expand Down
28 changes: 0 additions & 28 deletions eval/eval/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -672,31 +672,3 @@ cc_test(
"@com_google_googleapis//google/api/expr/v1alpha1:syntax_cc_proto",
],
)

cc_library(
name = "set_util",
srcs = ["set_util.cc"],
hdrs = ["set_util.h"],
copts = ["-std=c++14"],
deps = ["//eval/public:cel_value"],
)

cc_test(
name = "set_util_test",
size = "small",
srcs = [
"set_util_test.cc",
],
copts = ["-std=c++14"],
deps = [
":container_backed_list_impl",
":container_backed_map_impl",
":set_util",
"//eval/public:cel_value",
"//eval/public:unknown_set",
"@com_github_google_googletest//:gtest_main",
"@com_google_absl//absl/status",
"@com_google_absl//absl/time",
"@com_google_protobuf//:protobuf",
],
)
4 changes: 2 additions & 2 deletions eval/eval/function_step.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ bool ShouldAcceptOverload(const CelFunction* function,
if (function == nullptr) {
return false;
}
for (int i = 0; i < arguments.size(); i++) {
for (size_t i = 0; i < arguments.size(); i++) {
if (arguments[i].IsUnknownSet() || arguments[i].IsError()) {
return IsNonStrict(function->descriptor().name());
}
Expand All @@ -65,7 +65,7 @@ std::vector<CelValue> CheckForPartialUnknowns(
absl::Span<const AttributeTrail> attrs) {
std::vector<CelValue> result;
result.reserve(args.size());
for (int i = 0; i < args.size(); i++) {
for (size_t i = 0; i < args.size(); i++) {
auto attr_set = frame->unknowns_utility().CheckForUnknowns(
attrs.subspan(i, 1), /*use_partial=*/true);
if (!attr_set.attributes().empty()) {
Expand Down
1 change: 1 addition & 0 deletions eval/eval/function_step_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ std::string TestNameFn(testing::TestParamInfo<UnknownProcessingOptions> opt) {
case UnknownProcessingOptions::kAttributeAndFunction:
return "attribute_and_function";
}
return "";
}

INSTANTIATE_TEST_SUITE_P(
Expand Down
30 changes: 29 additions & 1 deletion eval/public/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ cc_library(
":cel_function",
":cel_options",
":cel_value",
"//eval/eval:set_util",
":set_util",
"@com_google_absl//absl/container:btree",
"@com_google_googleapis//google/api/expr/v1alpha1:syntax_cc_proto",
],
Expand Down Expand Up @@ -623,3 +623,31 @@ cc_test(
"@com_google_protobuf//:protobuf",
],
)

cc_library(
name = "set_util",
srcs = ["set_util.cc"],
hdrs = ["set_util.h"],
copts = ["-std=c++14"],
deps = ["//eval/public:cel_value"],
)

cc_test(
name = "set_util_test",
size = "small",
srcs = [
"set_util_test.cc",
],
copts = ["-std=c++14"],
deps = [
":cel_value",
":set_util",
":unknown_set",
"//eval/eval:container_backed_list_impl",
"//eval/eval:container_backed_map_impl",
"@com_github_google_googletest//:gtest_main",
"@com_google_absl//absl/status",
"@com_google_absl//absl/time",
"@com_google_protobuf//:protobuf",
],
)
13 changes: 9 additions & 4 deletions eval/public/activation.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,20 @@ class BaseActivation {
google::protobuf::Arena*) const = 0;

// Check whether a select path is unknown.
virtual bool IsPathUnknown(absl::string_view) const = 0;
virtual bool IsPathUnknown(absl::string_view) const { return false; }

// Return FieldMask defining the list of unknown paths.
virtual const google::protobuf::FieldMask unknown_paths() const = 0;
virtual const google::protobuf::FieldMask& unknown_paths() const {
return google::protobuf::FieldMask::default_instance();
}

// Return the collection of attribute patterns that determine "unknown"
// values.
virtual const std::vector<CelAttributePattern>& unknown_attribute_patterns()
const = 0;
const {
static const std::vector<CelAttributePattern> empty;
return empty;
}

virtual ~BaseActivation() {}
};
Expand Down Expand Up @@ -108,7 +113,7 @@ class Activation : public BaseActivation {
}

// Return FieldMask defining the list of unknown paths.
const google::protobuf::FieldMask unknown_paths() const override {
const google::protobuf::FieldMask& unknown_paths() const override {
return unknown_paths_;
}

Expand Down
1 change: 0 additions & 1 deletion eval/public/activation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ namespace runtime {
namespace {

using ::google::protobuf::Arena;
using testing::_;
using testing::ElementsAre;
using testing::Eq;
using testing::HasSubstr;
Expand Down
2 changes: 1 addition & 1 deletion eval/public/ast_traverse.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ namespace {

struct StackRecord {
public:
static const int kNotCallArg = -1;
static constexpr int kNotCallArg = -1;

StackRecord(const Expr *e, const SourceInfo *info)
: expr(e),
Expand Down
87 changes: 49 additions & 38 deletions eval/eval/set_util.cc → eval/public/set_util.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include "eval/eval/set_util.h"
#include "eval/public/set_util.h"

#include <algorithm>

Expand All @@ -11,42 +11,44 @@ namespace {
// Default implementation is operator<.
// Note: for UnknownSet, Error and Message, this is ptr less than.
template <typename T>
bool LessThanImpl(T lhs, T rhs) {
return lhs < rhs;
int ComparisonImpl(T lhs, T rhs) {
if (lhs < rhs) {
return -1;
} else if (lhs > rhs) {
return 1;
} else {
return 0;
}
}

// List specialization -- compare size then elementwise compare.
template <>
bool LessThanImpl(const CelList* lhs, const CelList* rhs) {
if (lhs->size() < rhs->size()) {
return true;
} else if (lhs->size() > rhs->size()) {
return false;
int ComparisonImpl(const CelList* lhs, const CelList* rhs) {
int size_comparison = ComparisonImpl(lhs->size(), rhs->size());
if (size_comparison != 0) {
return size_comparison;
}
for (int i = 0; i < lhs->size(); i++) {
CelValue lhs_i = lhs->operator[](i);
CelValue rhs_i = rhs->operator[](i);
if (CelValueLessThan(lhs_i, rhs_i)) {
return true;
}
if (CelValueLessThan(rhs_i, lhs_i)) {
return false;
int value_comparison = CelValueCompare(lhs_i, rhs_i);
if (value_comparison != 0) {
return value_comparison;
}
}
// equal
return false;
return 0;
}

// Map specialization -- size then sorted elementwise compare (i.e.
// <lhs_key_i, lhs_value_i> < <rhs_key_i, rhs_value_i>
//
// This is expensive, but hopefully maps will be rarely used in sets.
template <>
bool LessThanImpl(const CelMap* lhs, const CelMap* rhs) {
if (lhs->size() < rhs->size()) {
return true;
} else if (lhs->size() > rhs->size()) {
return false;
int ComparisonImpl(const CelMap* lhs, const CelMap* rhs) {
int size_comparison = ComparisonImpl(lhs->size(), rhs->size());
if (size_comparison != 0) {
return size_comparison;
}

std::vector<CelValue> lhs_keys;
Expand All @@ -65,47 +67,56 @@ bool LessThanImpl(const CelMap* lhs, const CelMap* rhs) {
std::sort(lhs_keys.begin(), lhs_keys.end(), &CelValueLessThan);
std::sort(rhs_keys.begin(), rhs_keys.end(), &CelValueLessThan);

for (int i = 0; i < lhs_keys.size(); i++) {
for (size_t i = 0; i < lhs_keys.size(); i++) {
auto lhs_key_i = lhs_keys[i];
auto rhs_key_i = rhs_keys[i];
if (CelValueLessThan(lhs_key_i, rhs_key_i)) {
return true;
}
if (CelValueLessThan(rhs_key_i, lhs_key_i)) {
return false;
int key_comparison = CelValueCompare(lhs_key_i, rhs_key_i);
if (key_comparison != 0) {
return key_comparison;
}

// keys equal, compare values.
auto lhs_value_i = lhs->operator[](lhs_key_i).value();
auto rhs_value_i = rhs->operator[](rhs_key_i).value();

if (CelValueLessThan(lhs_value_i, rhs_value_i)) {
return true;
}
if (CelValueLessThan(rhs_value_i, lhs_value_i)) {
return true;
int value_comparison = CelValueCompare(lhs_value_i, rhs_value_i);
if (value_comparison != 0) {
return value_comparison;
}
}
// maps equal
return false;
return 0;
}

struct LessThanVisitor {
struct ComparisonVisitor {
CelValue rhs;
LessThanVisitor(CelValue rhs) : rhs(rhs) {}
ComparisonVisitor(CelValue rhs) : rhs(rhs) {}
template <typename T>
bool operator()(T lhs_value) {
int operator()(T lhs_value) {
T rhs_value;
if (!rhs.GetValue(&rhs_value)) {
return CelValue::Type(CelValue::IndexOf<T>::value) < rhs.type();
return ComparisonImpl(CelValue::Type(CelValue::IndexOf<T>::value),
rhs.type());
}
return LessThanImpl(lhs_value, rhs_value);
return ComparisonImpl(lhs_value, rhs_value);
}
};

} // namespace

int CelValueCompare(CelValue lhs, CelValue rhs) {
return lhs.Visit<int>(ComparisonVisitor(rhs));
}

bool CelValueLessThan(CelValue lhs, CelValue rhs) {
return lhs.Visit<bool>(LessThanVisitor(rhs));
return lhs.Visit<int>(ComparisonVisitor(rhs)) < 0;
}

bool CelValueEqual(CelValue lhs, CelValue rhs) {
return lhs.Visit<int>(ComparisonVisitor(rhs)) == 0;
}

bool CelValueGreaterThan(CelValue lhs, CelValue rhs) {
return lhs.Visit<int>(ComparisonVisitor(rhs)) > 0;
}

} // namespace runtime
Expand Down
11 changes: 8 additions & 3 deletions eval/eval/set_util.h → eval/public/set_util.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef THIRD_PARTY_CEL_CPP_EVAL_EVAL_SET_UTIL_H_
#define THIRD_PARTY_CEL_CPP_EVAL_EVAL_SET_UTIL_H_
#ifndef THIRD_PARTY_CEL_CPP_EVAL_PUBLIC_SET_UTIL_H_
#define THIRD_PARTY_CEL_CPP_EVAL_PUBLIC_SET_UTIL_H_

#include "eval/public/cel_value.h"

Expand All @@ -25,14 +25,19 @@ namespace runtime {
// Note that for For messages, errors, and unknown sets, this is a ptr
// comparison.
bool CelValueLessThan(CelValue lhs, CelValue rhs);
bool CelValueEqual(CelValue lhs, CelValue rhs);
bool CelValueGreaterThan(CelValue lhs, CelValue rhs);
int CelValueCompare(CelValue lhs, CelValue rhs);

// Convenience alias for using the CelValueLessThan function in sets providing
// the stl interface.
using CelValueLessThanComparator = decltype(&CelValueLessThan);
using CelValueEqualComparator = decltype(&CelValueEqual);
using CelValueGreaterThanComparator = decltype(&CelValueGreaterThan);

} // namespace runtime
} // namespace expr
} // namespace api
} // namespace google

#endif // THIRD_PARTY_CEL_CPP_EVAL_EVAL_SET_UTIL_H_
#endif // THIRD_PARTY_CEL_CPP_EVAL_PUBLIC_SET_UTIL_H_
Loading

0 comments on commit b416ef7

Please sign in to comment.