-
Notifications
You must be signed in to change notification settings - Fork 81
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
Revise general explanation on type casting #1354
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Jaehyun Lee <99jaehyunlee@gmail.com>
At 2025-Jan-13 vlstill agreed to review in detail. No other LDWG members could think of reasons to object based on our discussion, but they should review and add their comments if they wish. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the new wording. For the sake of readability of this area of the spec I suggest some tweaks to the following text.
`(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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this sounds a little better?
`(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 | |
`(t) e`, where `t` is a type and `e` is an expression. Casts are only permitted in | |
cases 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: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the last sentence makes little less sense with the forward references. I suggest something like this:
section <<sec-implicit-casts>> for implicit casts. While this design is arguably | |
more onerous for programmers, it has several benefits: | |
section <<sec-implicit-casts>> for implicit casts. Main goals of this design are: |
and possibly add
- It limits implicit casts to the bare minimum.
* It reduces the number of cases that have to be considered in the P4 | ||
specification. Some targets may not support all casts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say the last one should be separate. and probably not a bulletpoint.
* It reduces the number of cases that have to be considered in the P4 | |
specification. Some targets may not support all casts. | |
* It reduces the number of cases that have to be considered in the P4 | |
specification. | |
Note: some targets may not support all casts. |
Following the discussion made at #1351, this PR revises the general explanation on type casting given in section 8.11. Casts* to the following:
Rather than stating "Casts are only permitted on base types and derived types introduced by
typedef
,type
, andenum
." (which is quite inaccurate), it puts forward references to the following subsections. Because:void
,error
,match_kind
, andstring
)struct
andheader
can be casted, e.g., at initialization.