Skip to content

Reject ternary/lpm/range match on error-typed table keys - #5676

Open
aeron-gh wants to merge 1 commit into
p4lang:mainfrom
aeron-gh:reject-numeric-match-on-error-key
Open

Reject ternary/lpm/range match on error-typed table keys#5676
aeron-gh wants to merge 1 commit into
p4lang:mainfrom
aeron-gh:reject-numeric-match-on-error-key

Conversation

@aeron-gh

@aeron-gh aeron-gh commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

The compiler currently accepts a table key of type error with a ternary, lpm or range match kind. Those match kinds interpret the key as a numeric value, mask or range, which has no meaning for an error value. Only exact and optional, which compare for equality, make sense for such a key.

This adds a check in the type checker that rejects ternary, lpm and range on an error key and tells the user to use exact or optional instead.

Tests:

  • testdata/p4_16_errors/issue5637.p4 covers the three rejected match kinds.
  • testdata/p4_16_samples/issue5637_legal_match_kinds.p4 confirms exact and optional on an error key still compile.
  • metrics_test_4.p4 had ternary on an error field, so it now uses optional.

This follows the LDWG discussion in p4lang/p4-spec#1389, which also raised the same question for bool keys (e.g. psa-bool-ternary-const-entry-bmv2.p4). I left bool untouched on purpose: that case is still open in the spec and has an accepted test relying on it, whereas the LDWG conclusion for error was to disallow it. If the spec later decides bool too, it can follow the same pattern.

Fixes #5637.

These match kinds treat the key as a numeric value or range, which has no meaning for a non-numeric error key. Only exact and optional are allowed.

Fixes p4lang#5637.

Signed-off-by: aeron-gh <agab0323@gmail.com>
@aeron-gh
aeron-gh force-pushed the reject-numeric-match-on-error-key branch from 6319691 to 6c57f3f Compare July 3, 2026 16:17
@aeron-gh

aeron-gh commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@ChrisDodd when you get a chance, mind taking a look at this one? It's a small type-checking change (10 lines in typeCheckStmt.cpp; the rest of the diff is the error test and its generated outputs), should be quick to review. Thanks!

@ChrisDodd

Copy link
Copy Markdown
Contributor

This feels like it should be target/arch specific (in the midend/backend) rather than in the target independent frontend. Can a target really not allow ternary matches on error? If the target uses a 1-hot binary representation for error codes internally, a ternary match might actually make sense (allows matching on a set of errors)

@aeron-gh

aeron-gh commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@ChrisDodd thanks for looking. This is coming from the Language side, not a target rule. It was raised as p4lang/p4-spec#1389, and the May 2026 LDWG concluded standard P4 should reject it (tracked as #5637), so I put the check next to the existing match-kind type check in the frontend.

The spec defines ternary as k & mask == value & mask, and lpm and range likewise need a numeric key. error is opaque and not serializable, so those operations aren't defined on it at the Language level, regardless of target. Your 1-hot case would work, but it depends on an internal representation the Language hides for error, so a source program can't portably rely on it. In standard P4 you'd match specific errors with exact entries, or optional for the value-or-wildcard case.

If you'd rather it not be a hard frontend rule, I'm happy to move it to the midend or gate it so a target can opt out.

@jafingerhut

Copy link
Copy Markdown
Contributor

This feels like it should be target/arch specific (in the midend/backend) rather than in the target independent frontend. Can a target really not allow ternary matches on error? If the target uses a 1-hot binary representation for error codes internally, a ternary match might actually make sense (allows matching on a set of errors)

I agree that if a target chose to define a specific bit pattern for parsing errors, they would likely want to do the kinds of things that you say (e.g. matching on specific bit positions).

But if a target did that, it would have to use a different type than error to represent the result of parsing, yes? e.g. the target would have to define a parsing result that was type bit<W> for some value W.

I understand that this means that type error defined in the language specification would likely be unused on that target, which seems fine to me. That is exactly what Tofino does, for example.

@jafingerhut

Copy link
Copy Markdown
Contributor

I did a little searching around the files defining the Tofino architectures, and note these things:

  • The verify function is not defined to take any arguments at all, not even a bool. It definitely is not defined to take a parameter with type error at all.
  • There is a type error defined, but as far as I can tell, it is not used anywhere.
  • There is a parser error with constants defined with type bit<16>.
  • There is a psa architecture defined for Tofino, but it uses a technique that seems to me not to correspond with the definition of core.p4 in the language specification, where it does 1typedef error ParserError_t;. I could not quickly find the definition of ParserError_t, but I would guess it would be bit<16>. This looks to me like an effort by hook or by crook to use the type named error`, but to try to enable Tofino hardware details like the 16-bit format of the parser error hardware field to be visible.

@aeron-gh

aeron-gh commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for digging into that, Andy. That matches what I'd expect: hardware that wants bit-level matching uses bit<16>, so the language error type having no ternary/lpm/range semantics is the right thing to enforce

@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Jul 6, 2026
@rcgoodfellow

Copy link
Copy Markdown

From LDWG discussion

  • Match cases on error that want "don't care" semantics but not ternary masking can use optional match type.
  • Architectures that want to have their own error semantics should have their own error types. Note that this requires changing the definition of extract.

Will have further discussion in August LDWG meeting.

@fruffy fruffy added the p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/). label Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Topics concerning the core segments of the compiler (frontend, midend, parser) p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

error type used as a ternary match type should be disallowed

5 participants