Allow constant folding of serializable enums of different types

[ChrisDodd]

Typechecking (and the P4 spec) allow comparing values of serializable enum types, however, ConstantFolding was hitting a BUG_CHECK when that happened with two constants of different serializable enums.

[vlstill]

Is the type guaranteed to have the same identity (pointer value) here?

[ChrisDodd]

It is not -- that's what the check is for. If it is NOT the same type, we need to compare the values of the enum instances (which is what will happen if we fall through and don't do this special-case handling of enums of the same type.)

[vlstill]

Sorry, my question was a bit too abridged. I mean to say: is the type pointer guaranteed to have the same identity if it represents same type (i.e. bit<8> on both side)? I think it is possible to create two different IR::Type_Bits instances with the same "value" that are nevertheless different pointers.

[vlstill]

Typechecking (and the P4 spec) allow comparing values of serializable enum types, however, ConstantFolding was hitting a BUG_CHECK when that happened with two constants of different serializable enums.

Frankly I don't see where the spec would allow comparison of enums of two different types. I think that would be the only place such comparison is allowed. There may be an argument that it is an implicit cast to the underlying type and then comparison (https://p4.org/wp-content/uploads/2024/10/P4-16-spec-v1.2.5.html#sec-implicit-casts):

For enums with an underlying type, it can be implicitly cast to its underlying type whenever appropriate, including but not limited to in shifts, concatenation, bit slicing indexes, header stack indexes as well as other unary and binary operations.

But this is a somewhat weird implicit cast as the resulting type is given by the fact it is an enum, not by it being used in a place where such a type would be expected. I don't think we have something like "overload resolution rules for operators" which would be the thing to clarify this in languages like C++ (or at least I am not aware of them). Maybe we should have such rules.

---

I am also not sure this is a good idea. Of course, the compiler should not crash, but I am not sure implicit comparison of two different enum types is a good idea because it could hide an error where someone accidentally compares a value to enum of different type.

[ChrisDodd]

I am also not sure this is a good idea. Of course, the compiler should not crash, but I am not sure implicit comparison of two different enum types is a good idea because it could hide an error where someone accidentally compares a value to enum of different type.

The spec says

an enum with an underlying type can be thought of as being a type derived from the underlying type carrying equality, assignment, and casts to/from the underlying type.

which implies you can use the enum type as if it is the underlying type. Maybe typechecking should be inserting casts here, but we avoid doing that currently because we don't want to eliminate the enum type (which is what having a cast would end up doing), as having the enum type is sometimes important to the control-plane/runtime interface generation (and it ends up exposed there sometimes). So figuring out exactly when to do that and when not to would be tricky.

[asl]

I am also not sure this is a good idea. Of course, the compiler should not crash, but I am not sure implicit comparison of two different enum types is a good idea because it could hide an error where someone accidentally compares a value to enum of different type.

I tend to agree and this could potentially lead to error-prone behavior. If there is explicit cast to the underlying type, then fine (e.g. (bit<2>)foo == (bit<2>)bar.Case. Implicit casts to underlying type is fine as well foo == 2w2, but just allowing foo == bar.Case looks quite problematic to me.