Parsing P4Info for Actions and type-checking params.

In this CL, InferAndCheckTypes was extended to perform type checking on action constraints in addition to table constraints, requiring action param info to be parsed and stored. This is part of the step to extend P4ToConstraintInfo to parse and store action constraints (see https://docs.google.com/document/d/11Nkn3_BAa43OX4I-_DPsoUokebGdRYx6QAk7eGVAyLc/edit?resourcekey=0-gO31LvkRyylb07KB1m3RMg#heading=h.8eovq8t5ess2)

PiperOrigin-RevId: 576945971

gutils/BUILD.bazel

@@ -36,7 +36,13 @@ cc_library(
         "proto.h",
     ],
     visibility = ["//visibility:public"],
-    deps = ["@com_google_protobuf//:protobuf"],
+    deps = [
+        ":source_location",
+        ":status",
+        "@com_google_absl//absl/status",
+        "@com_google_protobuf//:protobuf",
+        "@com_google_protobuf//src/google/protobuf/io",
+    ],
 )
 
 cc_library(

gutils/proto.cc

@@ -1,7 +1,17 @@
 #include "gutils/proto.h"
 
+#include <fcntl.h>
+
+#include <string>
+#include <string_view>
+
+#include "absl/status/status.h"
+#include "google/protobuf/io/zero_copy_stream_impl.h"
 #include "google/protobuf/message.h"
+#include "google/protobuf/text_format.h"
 #include "google/protobuf/util/message_differencer.h"
+#include "gutils/source_location.h"
+#include "gutils/status_builder.h"
 
 namespace gutils {
 
@@ -22,4 +32,29 @@ bool ProtoEqual(const google::protobuf::Message &message1,
   return ProtoEqual(message1, message2, differ);
 }
 
+absl::Status ReadProtoFromFile(std::string_view filename,
+                               google::protobuf::Message *message) {
+  // Verifies that the version of the library that we linked against is
+  // compatible with the version of the headers we compiled against.
+  /* copybara:insert(not needed nor possible in google3, as it is a mono repo)
+  GOOGLE_PROTOBUF_VERIFY_VERSION;
+  */
+
+  int fd = open(std::string(filename).c_str(), O_RDONLY);
+  if (fd < 0) {
+    return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC)
+           << "Error opening the file " << filename;
+  }
+
+  google::protobuf::io::FileInputStream file_stream(fd);
+  file_stream.SetCloseOnDelete(true);
+
+  if (!google::protobuf::TextFormat::Parse(&file_stream, message)) {
+    return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC)
+           << "Failed to parse file " << filename;
+  }
+
+  return absl::OkStatus();
+}
+
 }  // namespace gutils

gutils/proto.h

@@ -1,6 +1,9 @@
 #ifndef THIRD_PARTY_P4LANG_P4_CONSTRAINTS_GUTILS_PROTO_H_
 #define THIRD_PARTY_P4LANG_P4_CONSTRAINTS_GUTILS_PROTO_H_
 
+#include <string_view>
+
+#include "absl/status/status.h"
 #include "google/protobuf/message.h"
 #include "google/protobuf/util/message_differencer.h"
 
@@ -14,6 +17,10 @@ bool ProtoEqual(const google::protobuf::Message &message1,
 bool ProtoEqual(const google::protobuf::Message &message1,
                 const google::protobuf::Message &message2);
 
+// Read the contents of the file into a protobuf.
+absl::Status ReadProtoFromFile(std::string_view filename,
+                               google::protobuf::Message *message);
+
 }  //  namespace gutils
 
 #endif  // THIRD_PARTY_P4LANG_P4_CONSTRAINTS_GUTILS_PROTO_H_

p4_constraints/ast.proto

@@ -184,5 +184,10 @@ message SourceLocation {
     // If present, `line` and `column` are relative to an @entry_restriction
     // annotation attached to a table of the given name.
     string table_name = 4;
+
+    // P4 action name. Prefer `file_path` whenever possible.
+    // If present, `line` and `column` are relative to an @action_restriction
+    // annotation attached to an action of the given name.
+    string action_name = 5;
   }
 }

p4_constraints/backend/BUILD.bazel

@@ -1,4 +1,5 @@
 load("//e2e_tests:p4check.bzl", "cmd_diff_test")
+load("@com_github_p4lang_p4c//:bazel/p4_library.bzl", "p4_library")
 
 package(
     default_visibility = ["//visibility:public"],
@@ -83,6 +84,7 @@ cc_library(
         "//p4_constraints/frontend:constraint_kind",
         "//p4_constraints/frontend:parser",
         "@com_github_p4lang_p4runtime//:p4info_cc_proto",
+        "@com_github_p4lang_p4runtime//:p4types_cc_proto",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/log",
         "@com_google_absl//absl/log:check",
@@ -183,6 +185,20 @@ cc_library(
     ],
 )
 
+p4_library(
+    name = "p4_programs/action_restrictions_valid",
+    src = "p4_programs/action_restrictions_valid.p4",
+    p4info_out = "p4_programs/action_restrictions_valid.p4info.pb.txt",
+    visibility = ["//visibility:private"],
+)
+
+p4_library(
+    name = "p4_programs/action_restrictions_invalid",
+    src = "p4_programs/action_restrictions_invalid.p4",
+    p4info_out = "p4_programs/action_restrictions_invalid.p4info.pb.txt",
+    visibility = ["//visibility:private"],
+)
+
 cc_test(
     name = "symbolic_interpreter_test",
     srcs = ["symbolic_interpreter_test.cc"],
@@ -208,3 +224,22 @@ cc_test(
         "@com_google_googletest//:gtest_main",
     ],
 )
+
+cc_test(
+    name = "constraint_info_test",
+    srcs = ["constraint_info_test.cc"],
+    data = [
+        "p4_programs/action_restrictions_invalid.p4info.pb.txt",
+        "p4_programs/action_restrictions_valid.p4info.pb.txt",
+    ],
+    deps = [
+        ":constraint_info",
+        "//gutils:proto",
+        "//gutils:status_matchers",
+        "@com_github_p4lang_p4runtime//:p4info_cc_proto",
+        "@com_google_absl//absl/container:flat_hash_map",
+        "@com_google_absl//absl/log:check",
+        "@com_google_absl//absl/status:statusor",
+        "@com_google_googletest//:gtest_main",
+    ],
+)

p4_constraints/backend/constraint_info.cc

@@ -21,6 +21,7 @@
 #include <vector>
 
 #include "absl/container/flat_hash_map.h"
+#include "absl/log/log.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
@@ -30,6 +31,7 @@
 #include "absl/types/optional.h"
 #include "gutils/status_macros.h"
 #include "p4/config/v1/p4info.pb.h"
+#include "p4/config/v1/p4types.pb.h"
 #include "p4_constraints/ast.pb.h"
 #include "p4_constraints/backend/type_checker.h"
 #include "p4_constraints/constraint_source.h"
@@ -41,12 +43,43 @@ namespace p4_constraints {
 
 namespace {
 
+using p4::config::v1::Action;
+using p4::config::v1::Action_Param;
 using p4::config::v1::MatchField;
+using p4::config::v1::Preamble;
 using p4::config::v1::Table;
+using p4_constraints::ConstraintKind;
+
+RE2 GetConstraintAnnotation(ConstraintKind constraint_kind) {
+  switch (constraint_kind) {
+    case ConstraintKind::kTableConstraint:
+      return {R"RE(@entry_restriction)RE"};
+    case ConstraintKind::kActionConstraint:
+      return {R"RE(@action_restriction)RE"};
+  }
+  LOG(ERROR)
+      << "ConstraintKind is neither TableConstraint nor ActionConstraint";
+  return RE2("");
+}
+
+void SetConstraintLocationName(ConstraintKind constraint_kind,
+                               absl::string_view name,
+                               ast::SourceLocation& source_location) {
+  switch (constraint_kind) {
+    case ConstraintKind::kTableConstraint:
+      source_location.set_table_name(name);
+      return;
+    case ConstraintKind::kActionConstraint:
+      source_location.set_action_name(name);
+      return;
+  }
+  LOG(ERROR)
+      << "ConstraintKind is neither TableConstraint nor ActionConstraint";
+}
 
 absl::StatusOr<absl::optional<ConstraintSource>> ExtractConstraint(
-    const Table& table) {
-  // We expect .p4 files to have the following format:
+    ConstraintKind constraint_kind, const Preamble& preamble) {
+  // We expect .p4 files to have the following format for tables:
   // ```p4
   //   @file(__FILE__)              // optional
   //   @line(__LINE__)              // optional
@@ -55,16 +88,26 @@ absl::StatusOr<absl::optional<ConstraintSource>> ExtractConstraint(
   //   ")
   //   table foo { ... }
   // ```
-  // The @file/@line annotations are optional and intended for debugging/testing
-  // only; they allows us to give error messages that quote the source code.
+  // We expect .p4 files to have the following format for actions:
+  // ```p4
+  //   @file(__FILE__)              // optional
+  //   @line(__LINE__)              // optional
+  //   @action_restriction("
+  //      <the actual constraint>
+  //   ")
+  //   action bar { ... }
+  // ```
+  // The @file/@line annotations are optional and intended for
+  // debugging/testing only; they allows us to give error messages that quote
+  // the source code.
   const RE2 file_annotation = {R"RE(@file[(]"([^"]*)"[)])RE"};
   const RE2 line_annotation = {R"RE(@line[(](\d+)[)])RE"};
-  const RE2 constraint_annotation = {R"RE(@entry_restriction)RE"};
+  const RE2 constraint_annotation = GetConstraintAnnotation(constraint_kind);
 
   absl::string_view constraint_string = "";
   ast::SourceLocation constraint_location;
   int line = 0;
-  for (absl::string_view annotation : table.preamble().annotations()) {
+  for (absl::string_view annotation : preamble.annotations()) {
     if (RE2::Consume(&annotation, file_annotation,
                      constraint_location.mutable_file_path()))
       continue;
@@ -79,15 +122,18 @@ absl::StatusOr<absl::optional<ConstraintSource>> ExtractConstraint(
 
   constraint_location.set_line(line);
   if (constraint_location.file_path().empty()) {
-    constraint_location.set_table_name(table.preamble().name());
+    SetConstraintLocationName(constraint_kind, preamble.name(),
+                              constraint_location);
   }
 
   if (!absl::ConsumePrefix(&constraint_string, "(\"") ||
       !absl::ConsumeSuffix(&constraint_string, "\")")) {
+    bool is_table = constraint_kind == ConstraintKind::kTableConstraint;
     return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC)
-           << "In table " << table.preamble().name() << ":\n"
-           << "Syntax error: @entry_restriction must be enclosed in "
-              "'(\"' and '\")'";
+           << "In " << (is_table ? "table " : "action ") << preamble.name()
+           << ":\n"
+           << "Syntax error: @" << (is_table ? "entry" : "action")
+           << "_restriction must be enclosed in '(\"' and '\")'";
   }
   return ConstraintSource{
       .constraint_string = std::string(constraint_string),
@@ -129,6 +175,19 @@ absl::StatusOr<ast::Type> ParseKeyType(const MatchField& key) {
   }
 }
 
+absl::StatusOr<ast::Type> ParseParamType(const Action_Param& param) {
+  ast::Type type;
+  // P4NamedType is unset if the param does not use a user-defined type.
+  // Currently we do not support user-defined types.
+  if (!param.type_name().name().empty()) {
+    type.mutable_unsupported()->set_name(param.type_name().name());
+    return type;
+  }
+
+  type.mutable_fixed_unsigned()->set_bitwidth(param.bitwidth());
+  return type;
+}
+
 absl::StatusOr<TableInfo> ParseTableInfo(const Table& table) {
   absl::flat_hash_map<uint32_t, KeyInfo> keys_by_id;
   absl::flat_hash_map<std::string, KeyInfo> keys_by_name;
@@ -148,8 +207,9 @@ absl::StatusOr<TableInfo> ParseTableInfo(const Table& table) {
     }
   }
 
-  ASSIGN_OR_RETURN(absl::optional<ConstraintSource> constraint_source,
-                   ExtractConstraint(table));
+  ASSIGN_OR_RETURN(
+      absl::optional<ConstraintSource> constraint_source,
+      ExtractConstraint(ConstraintKind::kTableConstraint, table.preamble()));
 
   absl::optional<ast::Expression> constraint = absl::nullopt;
   if (constraint_source.has_value()) {
@@ -175,6 +235,52 @@ absl::StatusOr<TableInfo> ParseTableInfo(const Table& table) {
   return table_info;
 }
 
+absl::StatusOr<ActionInfo> ParseActionInfo(const Action& action) {
+  absl::flat_hash_map<uint32_t, ParamInfo> params_by_id;
+  absl::flat_hash_map<std::string, ParamInfo> params_by_name;
+
+  for (const Action_Param& param : action.params()) {
+    ASSIGN_OR_RETURN(const ast::Type type, ParseParamType(param));
+    const ParamInfo param_info{
+        .id = param.id(),
+        .name = param.name(),
+        .type = type,
+    };
+    if (!params_by_id.insert({param_info.id, param_info}).second) {
+      return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC)
+             << "action " << action.preamble().name()
+             << " has duplicate param: " << param.DebugString();
+    }
+    if (!params_by_name.insert({param_info.name, param_info}).second) {
+      return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC)
+             << "action " << action.preamble().name()
+             << " has duplicate param: " << param.DebugString();
+    }
+  }
+  ASSIGN_OR_RETURN(
+      absl::optional<ConstraintSource> constraint_source,
+      ExtractConstraint(ConstraintKind::kActionConstraint, action.preamble()));
+  absl::optional<ast::Expression> constraint;
+  if (constraint_source.has_value()) {
+    ASSIGN_OR_RETURN(
+        constraint,
+        ParseConstraint(ConstraintKind::kActionConstraint, *constraint_source));
+  }
+  ActionInfo action_info{
+      .id = action.preamble().id(),
+      .name = action.preamble().name(),
+      .constraint = constraint,
+      .constraint_source = constraint_source.value_or(ConstraintSource()),
+      .params_by_id = params_by_id,
+      .params_by_name = params_by_name,
+  };
+  // Type check constraint.
+  if (action_info.constraint.has_value()) {
+    RETURN_IF_ERROR(InferAndCheckTypes(&*action_info.constraint, action_info));
+  }
+  return action_info;
+}
+
 }  // namespace
 
 std::optional<AttributeInfo> GetAttributeInfo(
@@ -201,7 +307,6 @@ const TableInfo* GetTableInfoOrNull(const ConstraintInfo& constraint_info,
 absl::StatusOr<ConstraintInfo> P4ToConstraintInfo(
     const p4::config::v1::P4Info& p4info) {
   // Allocate output.
-  // TODO: b/293655979 - Populate the action_info_by_id map.
   absl::flat_hash_map<uint32_t, ActionInfo> action_info_by_id;
   absl::flat_hash_map<uint32_t, TableInfo> table_info_by_id;
 
@@ -217,6 +322,18 @@ absl::StatusOr<ConstraintInfo> P4ToConstraintInfo(
                        << "duplicate table: " << table.DebugString());
     }
   }
+
+  for (const Action& action : p4info.actions()) {
+    absl::StatusOr<ActionInfo> action_info = ParseActionInfo(action);
+    if (!action_info.ok()) {
+      errors.push_back(action_info.status());
+    } else if (!action_info_by_id.insert({action.preamble().id(), *action_info})
+                    .second) {
+      errors.push_back(gutils::InvalidArgumentErrorBuilder(GUTILS_LOC)
+                       << "duplicate action: " << action.DebugString());
+    }
+  }
+
   if (errors.empty()) {
     ConstraintInfo info{
         .action_info_by_id = std::move(action_info_by_id),