Export of internal changes (#49)

--
284576129:

BEGIN_PUBLIC
Define an abstract class for Activation.
This would be used by a Wasm context to implement FindValue, as opposed
to maintaining an embedded Activation. Values have unmanaged lifetimes,
meaning if it difficult to add/remove values from an Activation repeatedly.
END_PUBLIC
--
284290583:

Migrate existing users of the tools/build_defs/golden_test macros to use the new macro location.

This CL is supposed to be a no-op, and https://paste.googleplex.com/5747263567560704?raw shows it will be. If I've messed up, or e.g. an intervening depot CL means something unexpected happens, that's fine; please fix-forward.

This CL is #2 out of 3 in http://b/145011071#comment2.

Tested:
    TAP --sample ran all affected tests and none failed
    http://test/OCL:283986728:BASE:284250161:1575670327474:2f203dc8
--
284044257:

BEGIN_PUBLIC
Fix parsing bug with raw strings (e.g. r"").
Switch conformance tests to use C++ parser instead of Go parser.
END_PUBLIC

PiperOrigin-RevId: 284576129

Author: kyessenov

common/escaping.cc

@@ -266,7 +266,7 @@ absl::optional<std::string> unescape(const std::string& s, bool is_bytes) {
   // Raw string preceded by the 'r|R' prefix.
   bool is_raw_literal = false;
   if (value[0] == 'r' || value[0] == 'R') {
-    value.resize(value.size() - 1);
+    value = value.substr(1, n - 1);
     n = value.size();
     is_raw_literal = true;
   }

common/escaping_test.cc

@@ -35,6 +35,7 @@ std::vector<TestInfo> test_cases = {
     {R"("\\")", "\\"},
     {"'''x''x'''", "x''x"},
     {R"("""x""x""")", R"(x""x)"},
+    {R"(r"")", ""},
     // Octal 303 -> Code point 195 (Ã)
     // Octal 277 -> Code point 191 (¿)
     {R"("\303\277")", "ÿ"},

conformance/BUILD

@@ -38,11 +38,13 @@ cc_binary(
         "//eval/eval:container_backed_map_impl",
         "//eval/public:builtin_func_registrar",
         "//eval/public:cel_expr_builder_factory",
+        "//parser",
         "@com_github_grpc_grpc//:grpc++",
         "@com_google_absl//absl/strings",
         "@com_google_googleapis//google/api/expr/v1alpha1:checked_cc_proto",
         "@com_google_googleapis//google/api/expr/v1alpha1:conformance_service_cc_grpc",
         "@com_google_googleapis//google/api/expr/v1alpha1:syntax_cc_proto",
+        "@com_google_googleapis//google/rpc:code_cc_proto",
         "@com_google_protobuf//:protobuf",
     ],
 )
@@ -53,8 +55,8 @@ cc_binary(
         srcs = [":" + driver],
         args = [
             "$(location @com_google_cel_spec//tests/simple:simple_test)",
-            "--server=$(location @com_google_cel_go//server/main:cel_server)",
-            "--eval_server=$(location :server)",
+            "--server=$(location :server)",
+            "--check_server=$(location @com_google_cel_go//server/main:cel_server)",
             # Requires container support
             "--skip_test=basic/namespace/self_eval_container_lookup,self_eval_container_lookup_unchecked",
             # Requires heteregenous equality spec clarification
@@ -83,8 +85,8 @@ sh_test(
     srcs = ["@com_google_cel_spec//tests:conftest-nofail.sh"],
     args = [
         "$(location @com_google_cel_spec//tests/simple:simple_test)",
-        "--server=$(location @com_google_cel_go//server/main:cel_server)",
-        "--eval_server=$(location :server)",
+        "--server=$(location :server)",
+        "--check_server=$(location @com_google_cel_go//server/main:cel_server)",
     ] + ["$(location " + test + ")" for test in DASHBOARD_TESTS],
     data = [
         ":server",

conformance/server.cc

@@ -5,12 +5,14 @@
 #include "google/api/expr/v1alpha1/syntax.pb.h"
 #include "google/api/expr/v1alpha1/value.pb.h"
 #include "google/protobuf/struct.pb.h"
+#include "google/rpc/code.pb.h"
 #include "grpcpp/grpcpp.h"
 #include "absl/strings/str_split.h"
 #include "eval/eval/container_backed_list_impl.h"
 #include "eval/eval/container_backed_map_impl.h"
 #include "eval/public/builtin_func_registrar.h"
 #include "eval/public/cel_expr_builder_factory.h"
+#include "parser/parser.h"
 #include "base/statusor.h"
 
 
@@ -190,12 +192,25 @@ class ConformanceServiceImpl final
   Status Parse(grpc::ServerContext* context,
                const v1alpha1::ParseRequest* request,
                v1alpha1::ParseResponse* response) override {
-    return Status(StatusCode::UNIMPLEMENTED, "only Eval is supported");
+    if (request->cel_source().empty()) {
+      return Status(StatusCode::INVALID_ARGUMENT, "No source code.");
+    }
+    auto parse_status = parser::Parse(request->cel_source(), "");
+    if (!parse_status.ok()) {
+      auto issue = response->add_issues();
+      *issue->mutable_message() = std::string(parse_status.status().message());
+      issue->set_code(google::rpc::Code::INVALID_ARGUMENT);
+    } else {
+      google::api::expr::v1alpha1::ParsedExpr out;
+      out = parse_status.ValueOrDie();
+      response->mutable_parsed_expr()->CopyFrom(out);
+    }
+    return Status::OK;
   }
   Status Check(grpc::ServerContext* context,
                const v1alpha1::CheckRequest* request,
                v1alpha1::CheckResponse* response) override {
-    return Status(StatusCode::UNIMPLEMENTED, "only Eval is supported");
+    return Status(StatusCode::UNIMPLEMENTED, "Check is not supported");
   }
   Status Eval(grpc::ServerContext* context,
               const v1alpha1::EvalRequest* request,

eval/eval/evaluator_core.cc

@@ -18,12 +18,12 @@ const ExpressionStep* ExecutionFrame::Next() {
 }
 
 cel_base::StatusOr<CelValue> CelExpressionFlatImpl::Evaluate(
-    const Activation& activation, google::protobuf::Arena* arena) const {
+    const BaseActivation& activation, google::protobuf::Arena* arena) const {
   return Trace(activation, arena, CelEvaluationListener());
 }
 
 cel_base::StatusOr<CelValue> CelExpressionFlatImpl::Trace(
-    const Activation& activation, google::protobuf::Arena* arena,
+    const BaseActivation& activation, google::protobuf::Arena* arena,
     CelEvaluationListener callback) const {
   ExecutionFrame frame(&path_, activation, arena, max_iterations_);
 

eval/eval/evaluator_core.h

@@ -95,7 +95,7 @@ class ExecutionFrame {
   // flat is the flattened sequence of execution steps that will be evaluated.
   // activation provides bindings between parameter names and values.
   // arena serves as allocation manager during the expression evaluation.
-  ExecutionFrame(const ExecutionPath* flat, const Activation& activation,
+  ExecutionFrame(const ExecutionPath* flat, const BaseActivation& activation,
                  google::protobuf::Arena* arena, int max_iterations)
       : pc_(0UL),
         execution_path_(flat),
@@ -129,7 +129,7 @@ class ExecutionFrame {
   google::protobuf::Arena* arena() { return arena_; }
 
   // Returns reference to Activation
-  const Activation& activation() const { return activation_; }
+  const BaseActivation& activation() const { return activation_; }
 
   // Returns reference to iter_vars
   std::map<std::string, CelValue>& iter_vars() { return iter_vars_; }
@@ -151,7 +151,7 @@ class ExecutionFrame {
  private:
   size_t pc_;  // pc_ - Program Counter. Current position on execution path.
   const ExecutionPath* execution_path_;
-  const Activation& activation_;
+  const BaseActivation& activation_;
   ValueStack value_stack_;
   google::protobuf::Arena* arena_;
   const int max_iterations_;
@@ -173,11 +173,11 @@ class CelExpressionFlatImpl : public CelExpression {
       : path_(std::move(path)), max_iterations_(max_iterations) {}
 
   // Implementation of CelExpression evaluate method.
-  cel_base::StatusOr<CelValue> Evaluate(const Activation& activation,
+  cel_base::StatusOr<CelValue> Evaluate(const BaseActivation& activation,
                                     google::protobuf::Arena* arena) const override;
 
   // Implementation of CelExpression trace method.
-  cel_base::StatusOr<CelValue> Trace(const Activation& activation,
+  cel_base::StatusOr<CelValue> Trace(const BaseActivation& activation,
                                  google::protobuf::Arena* arena,
                                  CelEvaluationListener callback) const override;
 

eval/eval/function_step.cc

@@ -149,7 +149,7 @@ cel_base::StatusOr<const CelFunction*> LazyFunctionStep::ResolveFunction(
 
   CelFunctionDescriptor matcher{name_, receiver_style_, arg_types};
 
-  const Activation& activation = frame->activation();
+  const BaseActivation& activation = frame->activation();
   for (auto provider : providers_) {
     auto status = provider->GetFunction(matcher, activation);
     if (!status.ok()) {

eval/public/activation.h

@@ -17,26 +17,53 @@ namespace api {
 namespace expr {
 namespace runtime {
 
+// Base class for an activation.
+class BaseActivation {
+ public:
+  BaseActivation() = default;
+
+  // Non-copyable/non-assignable
+  BaseActivation(const BaseActivation&) = delete;
+  BaseActivation& operator=(const BaseActivation&) = delete;
+
+  // Return a list of function overloads for the given name.
+  virtual std::vector<const CelFunction*> FindFunctionOverloads(
+      absl::string_view) const = 0;
+
+  // Provide the value that is bound to the name, if found.
+  // arena parameter is provided to support the case when we want to pass the
+  // ownership of returned object ( Message/List/Map ) to Evaluator.
+  virtual absl::optional<CelValue> FindValue(absl::string_view,
+                                             google::protobuf::Arena*) const = 0;
+
+  // Check whether a select path is unknown.
+  virtual bool IsPathUnknown(absl::string_view) const = 0;
+
+  virtual ~BaseActivation() {}
+};
+
 // Instance of Activation class is used by evaluator.
 // It provides binding between references used in expressions
 // and actual values.
-class Activation {
+class Activation : public BaseActivation {
  public:
   Activation() = default;
 
   // Non-copyable/non-assignable
   Activation(const Activation&) = delete;
   Activation& operator=(const Activation&) = delete;
 
-  // Return list of function overloads for the given name.
+  // BaseActivation
   std::vector<const CelFunction*> FindFunctionOverloads(
-      absl::string_view name) const;
+      absl::string_view name) const override;
 
-  // Provide value that is bound to the name, if found.
-  // arena parameter is provided to support the case when we want to pass the
-  // ownership of returned object ( Message/List/Map ) to Evaluator.
   absl::optional<CelValue> FindValue(absl::string_view name,
-                                     google::protobuf::Arena* arena) const;
+                                     google::protobuf::Arena* arena) const override;
+
+  bool IsPathUnknown(absl::string_view path) const override {
+    return google::protobuf::util::FieldMaskUtil::IsPathInFieldMask(std::string(path),
+                                                          unknown_paths_);
+  }
 
   // Insert a function into the activation (ie a lazily bound function). Returns
   // a status if the name and shape of the function matches another one that has
@@ -75,12 +102,6 @@ class Activation {
     return unknown_paths_;
   }
 
-  // Check whether select path is unknown.
-  bool IsPathUnknown(absl::string_view path) const {
-    return google::protobuf::util::FieldMaskUtil::IsPathInFieldMask(std::string(path),
-                                                          unknown_paths_);
-  }
-
  private:
   class ValueEntry {
    public:

eval/public/cel_expression.h

@@ -34,12 +34,12 @@ class CelExpression {
   // activation contains bindings from parameter names to values
   // arena parameter specifies Arena object where output result and
   // internal data will be allocated.
-  virtual cel_base::StatusOr<CelValue> Evaluate(const Activation& activation,
+  virtual cel_base::StatusOr<CelValue> Evaluate(const BaseActivation& activation,
                                             google::protobuf::Arena* arena) const = 0;
 
   // Trace evaluates expression calling the callback on each sub-tree.
   virtual cel_base::StatusOr<CelValue> Trace(
-      const Activation& activation, google::protobuf::Arena* arena,
+      const BaseActivation& activation, google::protobuf::Arena* arena,
       CelEvaluationListener callback) const = 0;
 };
 

eval/public/cel_function_provider.cc

@@ -13,7 +13,7 @@ class ActivationFunctionProviderImpl : public CelFunctionProvider {
   ActivationFunctionProviderImpl() {}
   cel_base::StatusOr<const CelFunction*> GetFunction(
       const CelFunctionDescriptor& descriptor,
-      const Activation& activation) const override {
+      const BaseActivation& activation) const override {
     std::vector<const CelFunction*> overloads =
         activation.FindFunctionOverloads(descriptor.name());