p4lang / p4-spec

Apache License 2.0
178 stars 80 forks source link

Clarify explanation on casting types that are not base type and derived types introduced by typedef, type, and enum #1351

Open jaehyun1ee opened 6 days ago

jaehyun1ee commented 6 days ago

The spec mentions about casting at the beginning of section 8.11. Casts.

Casts are only permitted on base types and derived types introduced by typedef, type, and enum.

But in the description of explicit casts in section 8.11.1. Explicit Casts, we also consider the case that is not a base type nor a derived type introduced by typedef, type, and enum:

  • casts of a key-value list to a struct type or a header type (see Section 8.13)

And section 8.13. Operations on structure-valued expressions elaborates more:

expression ...
    | '{' kvList '}'
    | '(' typeRef ')' expression
    ;

For a structure-valued expression typeRef is the name of a struct or header type. The typeRef can be omitted if it can be inferred from context, e.g., when initializing a variable with a struct type.

Structure-valued expressions can be used in the right-hand side of assignments, in comparisons, in field selection expressions, and as arguments to functions, method or actions. Structure-valued expressions are not left values.

So, I believe there is some gap between what is claimed at the beginning of section 8.11 and what is explained in 8.11.1 and 8.13.

(1) Suggestion for revising 8.11

I propose changing the explanation in 8.11 to better reflect the below cases:

  • casts of a key-value list to a struct type or a header type (see Section 8.13)
  • casts of a tuple expression to a header stack type
  • casts of an invalid expression {#} to a header or a header union type

But currently I cannot think of a right phrase to capture this. As of my knowledge, the spec does not define an explicit type for key-value lists and tuple(list) expressions. So it doesn't nicely fit into the explanation:

(Current) Casts are only permitted on base types and derived types introduced by typedef, type, and enum. (Revised) Casts are only permitted on base types, (some nice way to phrase key-value list, list, and {#} types), and derived types introduced by typedef, type and enum.

(2) Relation to p4c test

Also, I wonder if this program (issue3233.p4) is in line with the current spec. This, when given to the p4c frontend, is rejected.

struct s {
  bit t;
  bit t1;
}

s func(bit t, bit t1) {
  return (s)(s)(s) { t, t1 };
}

s func1(s t1) {
  return (s)(s)(s) t1;
}
issue3233.p4(9): [--Werror=type-error] error: s: cast not supported
  return (s)(s)(s) { t, t1 };
             ^
issue3233.p4(14): [--Werror=type-error] error: s: cast not supported
  return (s)(s)(s) t1;
                ^

A related discussion was made in this issue.

But since the spec has mentioned that: casting a key-value list is valid, can we consider the above program as valid?

jafingerhut commented 4 hours ago

Below is one suggested wording for the revised sentence that you gave in your comment as:

Casts are only permitted on base types, (some nice way to phrase key-value list, list, and {#} types), and derived types introduced by typedef, type and enum.

I don't claim it is the best way, but I'm going for accuracy here:

Casts are only permitted on:

+ expressions whose type is a base type, or a derived type introduced by typedef, type, and enum
+ expressions that are syntactically one of:
  + a key-value list enclosed in curly braces (see non-terminal `kList` in the grammar), which can be part of a structure expression (see <<sec-structure-expressions>>), 
  + a tuple expression, enclosed in curly braces (see the expansion of grammar non-terminal `expression` into `"{" expressionList optTrailingComma "}"`), which can be used as part of:
    + a tuple: see <<sec-tuple-exprs>>
    + a header stack: see <<sec-hs-init>>
    + a list: see <<sec-list-exprs>>
  + an invalid header expression `{#}`: see <<sec-ops-on-hdrs>>

I think that might be an exhaustive list, but I might be missing something.

We could introduce a name for everything encompassed by the "expressions that are syntactically one of ..." and its sub-bullets, but it might not be worth the effort, if that name would only be used one place in the spec.

jaehyun1ee commented 58 minutes ago

Thank you for the suggestion :) I also think that the list is exhaustive.

On second thought, for the readability of the spec, introducing such a detailed explanation may be a bit redundant. Because section 8.11.1 immediately follows the explanation and also exhaustively listing all possible casts.

And I have also noticed that "casts are only permitted on base types" can also be inaccurate, in the sense that we do not allow casts on some base types, such as void, error, match_kind, and string types.

So how about this revision:

P4 provides a limited set of casts between types. A cast is written
`(t) e`, where `t` is a type and `e` is an expression. Casts are only permitted in
cases as defined in section <<sec-explicit-casts>> for explicit casts and
section <<sec-implicit-casts>> for implicit casts. While this design is arguably
more onerous for programmers, it has several benefits:

This explains that casts apply to limited occasions in P4, but puts a forward reference to the following subsections instead of listing them exhaustively.