Skip to content

Add op= assignments - #1359

Merged
jonathan-dilorenzo merged 2 commits into
p4lang:mainfrom
ChrisDodd:cdodd-opassign
Mar 13, 2025
Merged

Add op= assignments#1359
jonathan-dilorenzo merged 2 commits into
p4lang:mainfrom
ChrisDodd:cdodd-opassign

Conversation

@ChrisDodd

@ChrisDodd ChrisDodd commented Jan 30, 2025

Copy link
Copy Markdown
Contributor

For #1144

@ChrisDodd
ChrisDodd requested a review from jafingerhut January 30, 2025 01:53

@jafingerhut jafingerhut left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but probably best to let one of the co-chairs do final approval and merging, rather than merging with only my approval.

@vlstill vlstill left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The biggest question to me is what to do with booleans. And we should be careful to specify which operators apply here, not all binary operators are usable.

Comment thread p4-16/spec/P4-16-spec.adoc Outdated
Comment thread p4-16/spec/P4-16-spec.adoc Outdated
@ChrisDodd
ChrisDodd force-pushed the cdodd-opassign branch 2 times, most recently from a74548f to 178029a Compare February 4, 2025 01:42
@jafingerhut

Copy link
Copy Markdown
Collaborator

The biggest question to me is what to do with booleans. And we should be careful to specify which operators apply here, not all binary operators are usable.

@vlstill Are there any remaining concerns you have that have not been addressed? If so, please explain what they are, as the comments that I see appear to be addressed.

@vlstill vlstill left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably just add that this is allowed for integral types (bit<N>, int<N>, int).

@vlstill
vlstill dismissed their stale review March 10, 2025 15:29

Main concerns addressed.

@jafingerhut

Copy link
Copy Markdown
Collaborator

@jonathan-dilorenzo Discussed at 2025-Mar-10 LDWG meeting, and vlstill approved during the meeting. If you wish to do a final approval pass before merging, please do so, but we believe this is ready to merge into the spec. FYI, the implementation of this was merged into p4c recently.

@jonathan-dilorenzo

Copy link
Copy Markdown
Collaborator

@ChrisDodd, can I ask you to add a bullet point summarizing the change to this section before I merge?

Signed-off-by: Chris Dodd <cdodd@acm.org>
@ChrisDodd

Copy link
Copy Markdown
Contributor Author

@ChrisDodd, can I ask you to add a bullet point summarizing the change to this section before I merge?

done

Signed-off-by: Jonathan DiLorenzo <dilo@google.com>
@jonathan-dilorenzo
jonathan-dilorenzo merged commit c6af942 into p4lang:main Mar 13, 2025
@ChrisDodd
ChrisDodd deleted the cdodd-opassign branch July 23, 2025 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants