Skip to content

Commit

Permalink
Simplify API, returning StatusOr instead of status vector. (#57)
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 357261243

Co-authored-by: PINS Team <[email protected]>
  • Loading branch information
smolkaj and PINS Team authored Feb 12, 2021
1 parent 12d91dc commit 1662d96
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 28 deletions.
28 changes: 14 additions & 14 deletions e2e_tests/invalid_constraints.expected.output
Original file line number Diff line number Diff line change
@@ -1,63 +1,63 @@
In table invalid_constraints.forgot_quotes:
P4Info to constraint info translation failed with the following errors:
- In table invalid_constraints.forgot_quotes:
Syntax error: @entry_restriction must be enclosed in '("' and '")'

In table invalid_constraints.forgot_quotes_with_srcloc:
- In table invalid_constraints.forgot_quotes_with_srcloc:
Syntax error: @entry_restriction must be enclosed in '("' and '")'

In @entry_restriction of table invalid_constraints.empty_restriction; at offset line 1, column 1:
- In @entry_restriction of table invalid_constraints.empty_restriction; at offset line 1, column 1:
Parse error: unexpected token: <END_OF_INPUT>. Expected true, false, <BINARY>, <OCTARY>, <DECIMAL>, <HEXADEC>, <ID>, !, -, or (.

e2e_tests/invalid_constraints.p4:21:3:
- e2e_tests/invalid_constraints.p4:21:3:
| @entry_restriction("
21 | ")
| ^
Parse error: unexpected token: <END_OF_INPUT>. Expected true, false, <BINARY>, <OCTARY>, <DECIMAL>, <HEXADEC>, <ID>, !, -, or (.

e2e_tests/invalid_constraints.p4:27:5-15:
- e2e_tests/invalid_constraints.p4:27:5-15:
| @entry_restriction("
27 | foo.bar.baz == 2
| ^^^^^^^^^^^
Type error: unknown key foo.bar.baz

In @entry_restriction of table invalid_constraints.unknown_key_no_srcloc; at offset line 1, columns 1 to 11:
- In @entry_restriction of table invalid_constraints.unknown_key_no_srcloc; at offset line 1, columns 1 to 11:
Type error: unknown key foo.bar.baz

e2e_tests/invalid_constraints.p4:38:21-22:
- e2e_tests/invalid_constraints.p4:38:21-22:
| @entry_restriction("
38 | foo.bar.baz == 0b2
| ^^
Parse error: unexpected token: <ID>. Expected <END_OF_INPUT>, ), ::, &&, ;, ||, ->, ==, !=, >, >=, <, or <=.

In @entry_restriction of table invalid_constraints.bad_binary_numeral_no_srcloc; at offset line 1, columns 17 to 18:
- In @entry_restriction of table invalid_constraints.bad_binary_numeral_no_srcloc; at offset line 1, columns 17 to 18:
Parse error: unexpected token: <ID>. Expected <END_OF_INPUT>, ), ::, &&, ;, ||, ->, ==, !=, >, >=, <, or <=.

e2e_tests/invalid_constraints.p4:52:11-14:
- e2e_tests/invalid_constraints.p4:52:11-14:
| /**************************************************************************/
52 | error here
| ^^^^
Parse error: unexpected token: <ID>. Expected <END_OF_INPUT>, ), ::, &&, ;, ||, ->, ==, !=, >, >=, <, or <=.

e2e_tests/invalid_constraints.p4:60:5-15:
- e2e_tests/invalid_constraints.p4:60:5-15:
| @entry_restriction("
60 | ternary_key::prefix_length
| ^^^^^^^^^^^
Type error: expression of type ternary<32> has no field 'prefix_length'

e2e_tests/invalid_constraints.p4:70:5-10:
- e2e_tests/invalid_constraints.p4:70:5-10:
| @entry_restriction("
70 | 0x0F0F :: value
| ^^^^^^
Type error: expression of type int has no field 'value'

e2e_tests/invalid_constraints.p4:77:42-46:
- e2e_tests/invalid_constraints.p4:77:42-46:
| @entry_restriction("
77 | -0x0F0F == -0o01234567 -> !false && -false
| ^^^^^
Type error: expected type int, got bool

e2e_tests/invalid_constraints.p4:84:33:
- e2e_tests/invalid_constraints.p4:84:33:
| @entry_restriction("
84 | !false -> -8 == -0b1000 || !1
| ^
Type error: expected type bool, got int

13 changes: 10 additions & 3 deletions p4_constraints/backend/constraint_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "absl/container/flat_hash_map.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/str_join.h"
#include "absl/strings/string_view.h"
#include "absl/strings/strip.h"
#include "absl/types/variant.h"
Expand Down Expand Up @@ -160,7 +161,7 @@ absl::StatusOr<TableInfo> ParseTableInfo(const Table& table) {

} // namespace

absl::variant<ConstraintInfo, std::vector<absl::Status>> P4ToConstraintInfo(
absl::StatusOr<ConstraintInfo> P4ToConstraintInfo(
const p4::config::v1::P4Info& p4info) {
// Allocate output.
absl::flat_hash_map<uint32_t, TableInfo> info;
Expand All @@ -176,8 +177,14 @@ absl::variant<ConstraintInfo, std::vector<absl::Status>> P4ToConstraintInfo(
<< "duplicate table: " << table.DebugString());
}
}
if (!errors.empty()) return errors;
return info;
if (errors.empty()) return info;
return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC)
<< "P4Info to constraint info translation failed with the following "
"errors:\n- "
<< absl::StrJoin(errors, "\n- ",
[](std::string* out, const absl::Status& status) {
absl::StrAppend(out, status.message(), "\n");
});
}

} // namespace p4_constraints
5 changes: 3 additions & 2 deletions p4_constraints/backend/constraint_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#include "absl/container/flat_hash_map.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/types/variant.h"
#include "p4/config/v1/p4info.pb.h"
#include "p4_constraints/ast.pb.h"
Expand Down Expand Up @@ -65,8 +66,8 @@ using ConstraintInfo = absl::flat_hash_map<uint32_t, TableInfo>;
//
// Parses all tables and their constraint annotations into an in-memory
// representation suitable for constraint checking. Returns parsed
// representation, or a nonempty list of error statuses if parsing fails.
absl::variant<ConstraintInfo, std::vector<absl::Status>> P4ToConstraintInfo(
// representation, or an error statuses if parsing fails.
absl::StatusOr<ConstraintInfo> P4ToConstraintInfo(
const p4::config::v1::P4Info& p4info);

} // namespace p4_constraints
Expand Down
13 changes: 4 additions & 9 deletions p4_constraints/cli/p4check.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,11 @@ int main(int argc, char** argv) {
}

// Parse constraints and report potential errors.
absl::variant<ConstraintInfo, std::vector<absl::Status>> info_or_errors =
P4ToConstraintInfo(p4info);
if (absl::holds_alternative<std::vector<absl::Status>>(info_or_errors)) {
const auto& errors = absl::get<std::vector<absl::Status>>(info_or_errors);
for (const absl::Status& error : errors) {
std::cerr << error.message() << "\n\n";
}
absl::StatusOr<ConstraintInfo> constraint_info = P4ToConstraintInfo(p4info);
if (!constraint_info.ok()) {
std::cerr << constraint_info.status().message();
return 1;
}
const auto& constraint_info = absl::get<ConstraintInfo>(info_or_errors);

// Check table entries, if any where given.
for (const char* entry_filename :
Expand Down Expand Up @@ -133,7 +128,7 @@ int main(int argc, char** argv) {
entry.set_table_id(CoerceToTableId(entry.table_id()));

// Check entry.
absl::StatusOr<bool> result = EntryMeetsConstraint(entry, constraint_info);
absl::StatusOr<bool> result = EntryMeetsConstraint(entry, *constraint_info);
if (!result.ok()) {
std::cout << "Error: " << result.status() << "\n\n";
continue;
Expand Down

0 comments on commit 1662d96

Please sign in to comment.