-
Notifications
You must be signed in to change notification settings - Fork 82
Serializable enums: should comparison of values of two different enum types work? (or enum and underlying type) #1369
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
Comments
Discussed on P4 LDWG 2025-05-12 (with @rcgoodfellow, @jafingerhut, @ajwalton), we agreed that the comparison of enums of different types, and even enums with integers can be dangerous. We have also discussed the implicit cast from serializable enum type to its underlying type and tended to disallowing even that. Of course, that would be a breaking change, forcing addition of explicit casts into existing P4 programs that use these features. We have discussed potentially treating these as warnings first, before introducing them as errors in future version of P4 spec & P4C. Follow on:
Example of dangerous uses:
@vgurevich, by chance do you remember why the implicit casting and comparison for serializable enums were added? Do you know about uses cases that rely on it? |
The original intent behind serializable enums was to provide convenient named constants to the specific integer types. Nothing more, nothing less. I think that changing this direction is quite dangerous and unnecessary. We did have situations where we needed to allow flexibility/ Although If there is a desire to have safer enums, they can be derived from |
OK, thanks @vgurevich for filling in the details that noone on the LDWG remembered. So the original intent is basically a fancy Overall, modern languages tend to be more restrictive by default and have the less safe behavior (such as automatic casting/lifting of operations) opt-in (note that C++ is not a modern language in this regard). Maybe we might consider such approach for the P4-next and leave the current behavior for P4-16? I will still double-check the current P4C behavior in more details. |
@vlstill , you are correct -- the original intent was a fancy typedef with constants that can also be nicely displayed, e.g. in a CLI. As for the safety, let's think of it this way: the main reason we have serializable enums is because they can be put into the headers, is it not? (otherwise non-serializable would do, correct)? However, no matter how safe the language is made, it cannot control the contents of the headers it receives from the network, right? So, that means that either:
I hope that your choice will be (3) and that I didn't miss other important options. If we agree on that, then implicit casts between serializable enums and integers (and remember, they only work one way, i.e. from enum to integer, but not backwards) becomes a nice convenience feature that doesn't affect safety one way or another. |
Restricting an enumeration to a specific underlying type representation is not unique to P4. Rust, Java, Scala, Zig and many others have this ability. The entire point of doing this is allowing the enumeration to be used as a boundary between unstructured and structured data representation. Once the data is in a structured representation the portions of a program operating on that structured representation can depend on a well defined domain of values for that data.
Headers in P4 do not come out of nowhere, we are not just casting a pointer to a buffer onto a struct like you might in C. Headers are parsed. If an enumeration is part of a header, and the data from the wire cannot be parsed into a valid representation for that enum, then parsing has failed. In cases where unstructured data is permissible, modern lanaguages allow the compsition of enums to partition structured data from unstructured data in a way that requires the code consuming the enum to explicitly deal with unstructured cases (example). Compositional enums is something we are planning on proposing for p4-next. |
This is an interesting idea, but it doesn't seem to match the current definition of Specifically, assuming this code fragment:
Is the definition of the enum legit? (I think so) In tat case, what should exactly happen if the first nibble of the packet buffer contains something other than 4 or 6? Currently, the header will be extracted and the check will need to be done elsewhere. Do you think this is wrong? (I do not think so). Overall, from the very beginning P4 was designed to be a language where a C programmer would feel more-or-less at home. That was done on purpose, partially because of the background of the people who started it, but mostly because that makes total sense to do in a restricted environment. All behind-the scenes checks cost a lot. So, I would suggest that radical changes like this will go in a new generation of P4, which might be based on completely different principles. I also hope that we'll have the hardware to try those ideas on :) |
See discussion from p4lang/p4c#5246.
The question is if any of the case (1), (2), (3) should work.
As @ChrisDodd pointed out,
I would ready it as permitting (1) for sure and maybe (2), but not (3), because there the underlying type is different.
I think the case of different underlying type is clear (it is not allowed). Personally, I would prefer all of them being disallowed, but I am not sure if P4C actually disallows (1) currently, or only (2), so doing that might be a breaking change. Maybe at least a consideration for "P4-26".
The text was updated successfully, but these errors were encountered: