-
Notifications
You must be signed in to change notification settings - Fork 82
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 <[email protected]>
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.
p4-16/spec/P4-16-spec.adoc
Outdated
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.
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.
Thank you for the suggestion, I revised the sentences as:
Casts in P4 are permitted only under the conditions specified in section sec-explicit-casts for explicit casts and section sec-implicit-casts for implicit casts. By allowing casts only in limited scenarios, this restriction is more onerous for programmers, but it also provides several benefits.
I tried to make it explicit that casts are very limited in P4.
Signed-off-by: Jaehyun Lee <[email protected]>
By allowing casts only in limited scenarios, this restriction is more | ||
onerous for programmers, but it also provides 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.
nitpick: maybe this would read more naturally:
By allowing casts only in limited scenarios, this restriction is more | |
onerous for programmers, but it also provides several benefits. | |
Allowing casts only in limited scenarios is more onerous for | |
programmers, but it also provides several benefits. |
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.