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

Tuple fields #877

Merged
merged 4 commits into from Apr 2, 2021
Merged

Tuple fields #877

merged 4 commits into from Apr 2, 2021

Conversation

mihaibudiu
Copy link
Contributor

This small PR introduces tuple field extraction. Initially I was thinking to use a syntax like t.0 to extract a field, but using t.f0 seems equally good and requires no changes to the grammar, and will not introduce ambiguities.

@jafingerhut
Copy link
Contributor

Perhaps this is intended by design, but this proposal syntactically disallows the ability to have a run-time variable expression, or even a named compile-time constant (e.g. #define symbol, or const int x = 2;) as the index of the tuple element reference.

Using syntax like my_tuple[0] or similar, even if most or all implementations would likely limit the expression to a compile-time known value that is int (also bit<W> or int<W>), would enable code like:

#define MY_FAVORITE_INDEX 2
const int second_favorite_index = 5;

     a = my_tuple[MY_FAVORITE_INDEX];
     b = my_tuple[second_favorite_index];

@mihaibudiu
Copy link
Contributor Author

Yes, this is by design, with a dynamic index we cannot statically tell the field type.

@jafingerhut
Copy link
Contributor

So compile-time known value is needed, understood. But an expression still might be useful for developers? We allow compile-time known value expressions in bit slices today, and in place of the W in bit<W> types.

@mihaibudiu
Copy link
Contributor Author

This syntax won't work for expressions.

@mihaibudiu mihaibudiu mentioned this pull request Jun 25, 2020
@mihaibudiu
Copy link
Contributor Author

Fixes #864

@jnfoster
Copy link
Contributor

I approve but let's discuss at the July LDWG.

@liujed
Copy link
Member

liujed commented Dec 7, 2020

+1 for array indexing syntax, even if the indices are restricted to compile-time constant expressions.

@jnfoster
Copy link
Contributor

jnfoster commented Dec 7, 2020

Actually, upon reflection, I think array indexing is much better than f0, f1, etc.

@@ -3701,6 +3701,9 @@ list expressions.
tuple<bit<32>, bool> x = { 10, false };
~ End P4Example

The fields of a tuple are named `f0`, `f1`, etc,; they can be
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be "etc." not "etc,"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically one dot is for the etc abbreviation, and the second is the sentence-ending dot.

@@ -2486,7 +2486,7 @@ struct Parsed_headers {
### Tuple types { #sec-tuple-types }

A tuple is similar to a `struct`, in that it holds multiple
values. Unlike a `struct` type, tuples have no named fields. The
values. The fields of a tuple are named in order `f0`, `f1`, etc.. The
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be "etc." not "etc.."

Copy link
Contributor Author

@mihaibudiu mihaibudiu Dec 8, 2020

Choose a reason for hiding this comment

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

Let's see if I can implement an alternative prototype for this.

Choose a reason for hiding this comment

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

Technically one dot is for the etc abbreviation, and the second is the sentence-ending dot.

That might seem logical, but English defies logic in many ways. https://english.stackexchange.com/questions/8382/when-etc-is-at-the-end-of-a-phrase-do-you-place-a-period-after-it/8385#8385

Copy link
Contributor

Choose a reason for hiding this comment

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

@vgurevich
Copy link
Contributor

Would it make sense to introduce optional field names that can be defined by the programmer, while using f0, f1, etc. as a backup?

@mihaibudiu
Copy link
Contributor Author

@vgurevich can you elaborate on your proposal?

@vgurevich
Copy link
Contributor

vgurevich commented Dec 9, 2020

@mbudiu-vmw -- something like:

result = Hash.get({src_ip = hdr.ipv4.src_addr, dst_ip=hdr.ipv4.dst_addr, protocol=hdr.ipv4.proto});

@mihaibudiu
Copy link
Contributor Author

This is an anonymous struct, not a tuple. We have an issue filed for supporting anonymous structs, and you are listed as an owner.

@mihaibudiu
Copy link
Contributor Author

The spec should say whether tuple fields can be left values.

@onf-cla-manager
Copy link

Hi @mbudiu-vmw, this is the ONF bot 🤖 I'm glad you want to contribute to our projects! However, before accepting your contribution, we need to ask you to sign a Contributor License Agreement (CLA). You can do it online, it will take only a few minutes:

✒️ 👉 https://cla.opennetworking.org

After signing, make sure to add your Github user ID mbudiu-vmw to the agreement.

For more information or help:"
https://wiki.opennetworking.org/x/BgCUI

p4-16/spec/P4-16-spec.mdk Outdated Show resolved Hide resolved
@mihaibudiu
Copy link
Contributor Author

@jnfoster : made the requested typo fix. If you approve we can merge this.

@mihaibudiu mihaibudiu merged commit 47eda27 into master Apr 2, 2021
@mihaibudiu mihaibudiu deleted the tupleElim branch April 2, 2021 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants