Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify API, returning StatusOr instead of status vector. #57

Merged
merged 1 commit into from
Feb 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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