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

Duplicate match_kind declarations are allowed when they shoudn't #5085

Open
jaehyun1ee opened this issue Jan 6, 2025 · 3 comments
Open

Duplicate match_kind declarations are allowed when they shoudn't #5085

jaehyun1ee opened this issue Jan 6, 2025 · 3 comments
Labels
p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/).

Comments

@jaehyun1ee
Copy link

The spec mentions in 7.1.3. The match kind type that,

The match_kind type is very similar to the error type and is used to declare a set of distinct names that may be used in a table's key property (described in Section 14.2.1). All identifiers are inserted into the top-level namespace. It is an error to declare the same match_kind identifier multiple times.

However, p4c fails to reject a case when there is duplicate match_kind, for example,

match_kind { foo, bar }
match_kind { foo }

A related test case exists as pipe.p4 in testdata/p4_16_samples.

#include <core.p4>
match_kind {
    ternary,
    exact
} // core.p4 already defines those two

Also, this is in contrast with how the p4c frontend handles duplicate error declarations. p4c rejects duplicate error declarations. For example, below program is rejected.

error { FOO, BAR }
error { FOO }

// dup-error.p4(2): [--Werror=duplicate] error: FOO: Duplicates declaration FOO
@ChrisDodd
Copy link
Contributor

Both error and match_kind declarations should only appear in target arch includes, never in user programs, so these rules should really just be codifying what actual arhcitectures (try to) do.

@jaehyun1ee
Copy link
Author

Yes, but I believe it would be a good practice to have a dup-check for match_kinds as the frontend already does with error types, for the sake of checking the validity of arch files.

@fruffy fruffy added the p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/). label Jan 6, 2025
@vlstill
Copy link
Contributor

vlstill commented Jan 6, 2025

Yes, but I believe it would be a good practice to have a dup-check for match_kinds as the frontend already does with error types, for the sake of checking the validity of arch files.

I agree, the check should not be omitted just because architecture programmers are considered "more trusted".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/).
Projects
None yet
Development

No branches or pull requests

4 participants