Skip to content
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

Add definition for compound assignment operators #1144

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rst0git
Copy link
Member

@rst0git rst0git commented Sep 9, 2022

This pull request adds definition for compound-assignment operators to the P4 language specification. These operators provide a shorter syntax for assigning the result of an arithmetic or bitwise operator. The proposed definition of compound assignment operators is similar the C99 specification.

@rst0git rst0git changed the title Add definition for compound assignment Add definition for compound assignment operators Sep 9, 2022
@rst0git rst0git force-pushed the shorthand-assignment-operators branch from 07c5b42 to 440d1c9 Compare September 9, 2022 13:47
@vgurevich
Copy link
Contributor

Did we intentionally forget about |+| and |-| operations? :)

@rst0git rst0git force-pushed the shorthand-assignment-operators branch from 440d1c9 to 5ab618a Compare September 10, 2022 23:05
@rst0git
Copy link
Member Author

rst0git commented Sep 10, 2022

Did we intentionally forget about |+| and |-| operations? :)

Thank you Vladimir for pointing this out! I have revised the pull request to include saturating arithmetic operations.

@rst0git rst0git force-pushed the shorthand-assignment-operators branch from 5ab618a to 680a6e2 Compare September 10, 2022 23:14
@apinski-cavium
Copy link
Contributor

Are the "+=" operators two tokens or just one?
That is can you do:
a + = b;
or does it need to be without whitespaces?
a += b;

The earlier version only needs parser changes while the later needs both lexer and parser changes.
Also it would nice to update the grammar in the Appendix for these too.

@mihaibudiu
Copy link
Contributor

Not only in the appendix.
We should also make sure that this won't introduce conflicts in the bison parser (mostly likely not).
The implementation has some subtle points - do we want to eliminate these early, in a midend pass, or not at all?
If we eliminate them early in the frontend most passes won't need to learn about them.
Otherwise we have to add code for handling them in many other places.

@apinski-cavium
Copy link
Contributor

We should also make sure that this won't introduce conflicts in the bison parser (mostly likely not).

It won't because statements are defined as:
lvalue = expression
lvalue ( ...

And lvalue does not have any of those tokens in it.
If it was a single token it would make my parser easier though because I actually handle
expression = expression
as a way to handle things better for error reasons.

@rst0git rst0git force-pushed the shorthand-assignment-operators branch 3 times, most recently from 9af9ac5 to 7fbc33b Compare September 13, 2022 20:39
@rst0git rst0git force-pushed the shorthand-assignment-operators branch from 7fbc33b to 6c58440 Compare September 17, 2022 11:02
p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
This patch adds definition for compound-assignment operators to the P4
language specification. These operators provide a shorter syntax for
assigning the result of an arithmetic or bitwise operator. The proposed
definition of compound assignment operators is similar the
C99 specification.

https://en.cppreference.com/w/c/language/operator_assignment

Signed-off-by: Radostin Stoyanov <radostin.stoyanov@eng.ox.ac.uk>
Copy link
Collaborator

@jafingerhut jafingerhut left a comment

Choose a reason for hiding this comment

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

LGTM

@jnfoster
Copy link
Collaborator

In the interest of tidying up the set of active issues on the P4 specification repository, I'm marking this as "stalled" and closing it. Of course, we can always re-open it in the future if there is interest in resurrecting it.

@jnfoster jnfoster closed this Nov 11, 2023
@jafingerhut jafingerhut reopened this Jan 22, 2025
@jafingerhut
Copy link
Collaborator

Reopening due to new interest in implementing this in p4c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants