[p4-constraints] Extend ParseConstraint to handle action_constraint p…

…arsing. (#112) Co-authored-by: PINS Team <[email protected]>
p4lang · Oct 24, 2023 · c42adaa · c42adaa
1 parent d054e74
commit c42adaa
Show file tree

Hide file tree

Showing 19 changed files with 174 additions and 51 deletions.
diff --git a/p4_constraints/BUILD.bazel b/p4_constraints/BUILD.bazel
@@ -39,6 +39,7 @@ cc_test(
         ":ast",
         "//gutils:parse_text_proto",
         "//gutils:status_matchers",
+        "@com_google_absl//absl/container:flat_hash_set",
         "@com_google_absl//absl/strings",
         "@com_google_googletest//:gtest_main",
     ],

diff --git a/p4_constraints/ast.cc b/p4_constraints/ast.cc
@@ -14,8 +14,6 @@
 
 #include "p4_constraints/ast.h"
 
-#include <stdint.h>
-
 #include <string>
 
 #include "absl/container/flat_hash_set.h"
@@ -214,6 +212,8 @@ void AddMatchFields(const ast::Expression& expr,
     case ast::Expression::kKey:
       field_set.insert(expr.key());
       return;
+    case ast::Expression::kActionParameter:
+      return;
     case ast::Expression::kBooleanNegation:
       AddMatchFields(expr.boolean_negation(), field_set);
       return;

diff --git a/p4_constraints/ast.proto b/p4_constraints/ast.proto
@@ -36,6 +36,8 @@ message Expression {
     string integer_constant = 5;
     // A table key (aka "match field"), e.g. `header.ethernet.ether_type`.
     string key = 6;
+    // An action parameter.
+    string action_parameter = 13;
     Expression boolean_negation = 7;
     Expression arithmetic_negation = 8;
     // Type casts are not exposed in the surface language, but may be inserted

diff --git a/p4_constraints/ast_test.cc b/p4_constraints/ast_test.cc
@@ -19,6 +19,7 @@
 
 #include <string>
 
+#include "absl/container/flat_hash_set.h"
 #include "absl/strings/substitute.h"
 #include "gutils/parse_text_proto.h"
 #include "gutils/status_matchers.h"
@@ -293,5 +294,16 @@ TEST(SizeTest, NoCacheOkay) {
   EXPECT_THAT(Size(expr, nullptr), IsOkAndHolds(Eq(7)));
 }
 
+TEST(AddMatchFields, ReturnsEmptyForActionParameter) {
+  Expression expr = ParseRawAst(R"pb(
+    binary_expression {
+      binop: GE
+      left { action_parameter: "1" }
+      right { action_parameter: "2" }
+    }
+  )pb");
+  EXPECT_THAT(GetMatchFields(expr), testing::IsEmpty());
+}
+
 }  // namespace ast
 }  // namespace p4_constraints
diff --git a/p4_constraints/backend/BUILD.bazel b/p4_constraints/backend/BUILD.bazel
@@ -25,7 +25,6 @@ cc_library(
         "//p4_constraints:constraint_source",
         "//p4_constraints:quote",
         "@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
-        "@com_google_absl//absl/container:btree",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/container:flat_hash_set",
         "@com_google_absl//absl/log",
@@ -82,6 +81,7 @@ cc_library(
         "//p4_constraints:ast_cc_proto",
         "//p4_constraints:constraint_source",
         "//p4_constraints:quote",
+        "//p4_constraints/frontend:constraint_kind",
         "//p4_constraints/frontend:parser",
         "@com_github_p4lang_p4runtime//:p4info_cc_proto",
         "@com_google_absl//absl/container:flat_hash_map",
@@ -108,9 +108,8 @@ cc_test(
         "//gutils:parse_text_proto",
         "//gutils:status",
         "//p4_constraints:constraint_source",
+        "//p4_constraints/frontend:constraint_kind",
         "//p4_constraints/frontend:parser",
-        "@com_google_absl//absl/container:btree",
-        "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/log:check",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
@@ -198,6 +197,7 @@ cc_test(
         "//gutils:testing",
         "//p4_constraints:ast_cc_proto",
         "//p4_constraints:constraint_source",
+        "//p4_constraints/frontend:constraint_kind",
         "//p4_constraints/frontend:parser",
         "@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
         "@com_github_z3prover_z3//:api",

diff --git a/p4_constraints/backend/constraint_info.cc b/p4_constraints/backend/constraint_info.cc
@@ -33,6 +33,7 @@
 #include "p4_constraints/ast.pb.h"
 #include "p4_constraints/backend/type_checker.h"
 #include "p4_constraints/constraint_source.h"
+#include "p4_constraints/frontend/constraint_kind.h"
 #include "p4_constraints/frontend/parser.h"
 #include "re2/re2.h"
 
@@ -152,7 +153,9 @@ absl::StatusOr<TableInfo> ParseTableInfo(const Table& table) {
 
   absl::optional<ast::Expression> constraint = absl::nullopt;
   if (constraint_source.has_value()) {
-    ASSIGN_OR_RETURN(constraint, ParseConstraint(*constraint_source));
+    ASSIGN_OR_RETURN(
+        constraint,
+        ParseConstraint(ConstraintKind::kTableConstraint, *constraint_source));
   }
 
   TableInfo table_info{

diff --git a/p4_constraints/backend/interpreter.cc b/p4_constraints/backend/interpreter.cc
@@ -39,7 +39,6 @@
 #include "gutils/ordered_map.h"
 #include "gutils/overload.h"
 #include "gutils/ret_check.h"
-#include "gutils/status.h"
 #include "gutils/status_macros.h"
 #include "p4/v1/p4runtime.pb.h"
 #include "p4_constraints/ast.h"
@@ -637,6 +636,11 @@ absl::StatusOr<EvalResult> Eval_(const Expression& expr,
       return it->second;
     }
 
+    case Expression::kActionParameter: {
+      return absl::UnimplementedError(
+          "TODO: b/293656077 - Support action constraints");
+    }
+
     case Expression::kAttributeAccess: {
       const std::string attribute_name =
           expr.attribute_access().attribute_name();

diff --git a/p4_constraints/backend/interpreter_golden_test_runner.cc b/p4_constraints/backend/interpreter_golden_test_runner.cc
@@ -16,12 +16,9 @@
 // interpreter.cc. Expected output is `interpreter_golden_test_runner.expected`
 
 #include <iostream>
-#include <optional>
 #include <string>
 #include <vector>
 
-#include "absl/container/btree_map.h"
-#include "absl/container/flat_hash_map.h"
 #include "absl/log/check.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
@@ -35,6 +32,7 @@
 #include "p4_constraints/backend/interpreter.h"
 #include "p4_constraints/backend/type_checker.h"
 #include "p4_constraints/constraint_source.h"
+#include "p4_constraints/frontend/constraint_kind.h"
 #include "p4_constraints/frontend/parser.h"
 
 namespace p4_constraints {
@@ -87,7 +85,8 @@ absl::StatusOr<ConstraintInfo> MakeConstraintInfo(TestCase test_case) {
 
   TableInfo table_info = kTableInfo;
   table_info.constraint_source = source;
-  ASSIGN_OR_RETURN(Expression expression, ParseConstraint(source));
+  ASSIGN_OR_RETURN(Expression expression,
+                   ParseConstraint(ConstraintKind::kTableConstraint, source));
   CHECK_OK(InferAndCheckTypes(&(expression), kTableInfo));
   table_info.constraint = expression;
   return ConstraintInfo{{table_info.id, table_info}};

diff --git a/p4_constraints/backend/symbolic_interpreter.cc b/p4_constraints/backend/symbolic_interpreter.cc
@@ -355,6 +355,11 @@ absl::StatusOr<SymbolicEvalResult> EvalSymbolically(
       return *key;
     }
 
+    case ast::Expression::kActionParameter: {
+      return absl::UnimplementedError(
+          "TODO: b/293656077 - Support action constraints");
+    }
+
     case ast::Expression::kFieldAccess: {
       // There are no nested field accesses supported in P4-Constraints at the
       // moment. If there were, this logic would need to change.

diff --git a/p4_constraints/backend/symbolic_interpreter_test.cc b/p4_constraints/backend/symbolic_interpreter_test.cc
@@ -24,6 +24,7 @@
 #include "p4_constraints/backend/interpreter.h"
 #include "p4_constraints/backend/type_checker.h"
 #include "p4_constraints/constraint_source.h"
+#include "p4_constraints/frontend/constraint_kind.h"
 #include "p4_constraints/frontend/parser.h"
 #include "z3++.h"
 
@@ -508,7 +509,8 @@ TableInfo GetTableInfoWithConstraint(absl::string_view constraint_string) {
           },
   };
 
-  auto constraint = ParseConstraint(table_info.constraint_source);
+  auto constraint = ParseConstraint(ConstraintKind::kTableConstraint,
+                                    table_info.constraint_source);
   CHECK_OK(constraint);
   CHECK_OK(InferAndCheckTypes(&(*constraint), table_info));
   table_info.constraint = *constraint;

diff --git a/p4_constraints/backend/type_checker.cc b/p4_constraints/backend/type_checker.cc
@@ -249,6 +249,11 @@ absl::Status InferAndCheckTypes(Expression* expr, const TableInfo& table_info) {
       return absl::OkStatus();
     }
 
+    case Expression::kActionParameter: {
+      return absl::UnimplementedError(
+          "TODO: b/293656077 - Support action constraints");
+    }
+
     case Expression::kAttributeAccess: {
       const std::string& attribute_name =
           expr->attribute_access().attribute_name();

diff --git a/p4_constraints/backend/type_checker_test.cc b/p4_constraints/backend/type_checker_test.cc
@@ -91,6 +91,7 @@ class InferAndCheckTypesTest : public ::testing::Test {
       case ast::Expression::kBooleanConstant:
       case ast::Expression::kIntegerConstant:
       case ast::Expression::kKey:
+      case ast::Expression::kActionParameter:
       case ast::Expression::kAttributeAccess:
       case ast::Expression::EXPRESSION_NOT_SET:
         return;

diff --git a/p4_constraints/frontend/BUILD.bazel b/p4_constraints/frontend/BUILD.bazel
@@ -9,6 +9,7 @@ cc_library(
     visibility = ["//visibility:public"],
     deps = [
         ":ast_constructors",
+        ":constraint_kind",
         ":lexer",
         ":token",
         "//gutils:status",
@@ -27,13 +28,13 @@ cc_test(
     size = "small",
     srcs = ["parser_test.cc"],
     deps = [
+        ":constraint_kind",
         ":parser",
         ":token",
         "//gutils:protocol_buffer_matchers",
         "//gutils:status_matchers",
         "//p4_constraints:ast_cc_proto",
         "//p4_constraints:constraint_source",
-        "@com_google_absl//absl/status:statusor",
         "@com_google_googletest//:gtest_main",
     ],
 )
@@ -101,6 +102,7 @@ cc_library(
     ],
     visibility = ["//p4_constraints/backend:__pkg__"],
     deps = [
+        ":constraint_kind",
         ":token",
         "//gutils:ret_check",
         "//gutils:status",
@@ -109,3 +111,9 @@ cc_library(
         "@com_google_absl//absl/types:span",
     ],
 )
+
+cc_library(
+    name = "constraint_kind",
+    hdrs = ["constraint_kind.h"],
+    visibility = ["//p4_constraints/backend:__pkg__"],
+)
diff --git a/p4_constraints/frontend/ast_constructors.cc b/p4_constraints/frontend/ast_constructors.cc
@@ -25,6 +25,7 @@
 #include "gutils/ret_check.h"
 #include "gutils/status_macros.h"
 #include "p4_constraints/ast.pb.h"
+#include "p4_constraints/frontend/constraint_kind.h"
 #include "p4_constraints/frontend/token.h"
 
 namespace p4_constraints {
@@ -136,18 +137,31 @@ absl::StatusOr<ast::Expression> MakeArithmeticNegation(
   return ast;
 }
 
-absl::StatusOr<ast::Expression> MakeKey(absl::Span<const Token> key_fragments) {
-  RET_CHECK_GT(key_fragments.size(), 0);
-  ast::Expression ast = LocatedExpression(key_fragments.front().start_location,
-                                          key_fragments.back().end_location);
-  std::stringstream key{};
-  for (int i = 0; i < key_fragments.size(); i++) {
-    const Token& id = key_fragments[i];
+absl::StatusOr<ast::Expression> MakeVariable(absl::Span<const Token> tokens,
+                                             ConstraintKind constraint_kind) {
+  RET_CHECK_GT(tokens.size(), 0);
+  ast::Expression ast = LocatedExpression(tokens.front().start_location,
+                                          tokens.back().end_location);
+  std::stringstream key_or_param{};
+  for (int i = 0; i < tokens.size(); i++) {
+    const Token& id = tokens[i];
     RET_CHECK_EQ(id.kind, Token::ID);
-    key << (i == 0 ? "" : ".") << id.text;
+    if (constraint_kind == ConstraintKind::kTableConstraint)
+      key_or_param << (i == 0 ? "" : ".") << id.text;
   }
-  ast.set_key(key.str());
-  return ast;
+  switch (constraint_kind) {
+    case ConstraintKind::kTableConstraint: {
+      ast.set_key(key_or_param.str());
+      return ast;
+    }
+    case ConstraintKind::kActionConstraint: {
+      ast.set_action_parameter(key_or_param.str());
+      return ast;
+    }
+  }
+  return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC)
+         << "Unexpected value for ConstraintKind: "
+         << static_cast<int>(constraint_kind);
 }
 
 absl::StatusOr<ast::Expression> MakeAttributeAccess(

diff --git a/p4_constraints/frontend/ast_constructors.h b/p4_constraints/frontend/ast_constructors.h
@@ -23,6 +23,7 @@
 #include "absl/status/statusor.h"
 #include "absl/types/span.h"
 #include "p4_constraints/ast.pb.h"
+#include "p4_constraints/frontend/constraint_kind.h"
 #include "p4_constraints/frontend/token.h"
 
 namespace p4_constraints {
@@ -39,9 +40,12 @@ absl::StatusOr<ast::Expression> MakeIntegerConstant(const Token& numeral);
 absl::StatusOr<ast::Expression> MakeAttributeAccess(
     const Token& double_colon, const Token& attribute_name);
 
-// Returns an AST `a` such that `a.key() == "id1.id2...idn"` if given ID tokens
-// `{t1, ..., tn}` such that `idi == ti.text`, or an error Status otherwise.
-absl::StatusOr<ast::Expression> MakeKey(absl::Span<const Token> key_fragments);
+// Returns an AST `a` such that `a.param() == "id1id2...idn"` (if parsing action
+// parameters) and `a.key() == "id1.id2...idn"` (if parsing keys) given ID
+// tokens `{t1, ..., tn}` such that `idi == ti.text`, or an error Status
+// otherwise.
+absl::StatusOr<ast::Expression> MakeVariable(absl::Span<const Token> tokens,
+                                             ConstraintKind constraint_kind);
 
 // Returns an AST (with the given operand) when given a BANG ('!') token,
 // or an error Status otherwise.

diff --git a/p4_constraints/frontend/constraint_kind.h b/p4_constraints/frontend/constraint_kind.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2023 The P4-Constraints Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef P4_CONSTRAINTS_FRONTEND_CONSTRAINT_KIND_H_
+#define P4_CONSTRAINTS_FRONTEND_CONSTRAINT_KIND_H_
+
+namespace p4_constraints {
+
+enum class ConstraintKind {
+  // Constraint attached to a P4 table using an `@entry_restriction` annotation.
+  kTableConstraint,
+  // Constraint attached to a P4 action using an `@action_restriction`
+  // annotation.
+  kActionConstraint,
+};
+
+}  // namespace p4_constraints
+
+#endif  // P4_CONSTRAINTS_FRONTEND_CONSTRAINT_KIND_H_