hampi icon indicating copy to clipboard operation
hampi copied to clipboard

ASN.1 compiler deriving Eq support for ASN.1 REAL values causes Rust compiler errors

Open gth828r opened this issue 1 year ago • 5 comments

Now that the ASN.1 REAL type is supported as an f64, there is a new issue.

The derive macros for the code generation tool are set at a global level. Deriving the Eq trait is likely common for some of the generated types; however, f64 does not support the Eq trait, only PartialEq.

There needs to be some way to generate code such that the Eq trait is not generated for REAL valued types.

gth828r avatar Nov 07 '23 21:11 gth828r

One simple fix would be to simply skip the Eq trait when the value has type REAL. There might be other more nuanced approaches as well, but I have not thought much about it yet.

gth828r avatar Nov 07 '23 21:11 gth828r

On second thought, this is slightly more complicated. Any parent type that includes REAL fields must also not set the Eq trait, so there is a cascading effect here.

For now, I think I personally can get away with only using the PartialEq trait for specs that include REAL fields, so this is not urgent on my end.

gth828r avatar Nov 08 '23 00:11 gth828r

Both Eq and PartialEq are opt-in traits. So one way to deal with this would be to just derive PartialEq by default during build.rs. An example of how to do it is available inside the build.rs file. There might be nuances that are not handled in the code yet (eg. IIRC: If you derive a Eq, PartialEq is required to be derived as well (or the other way round)).

Yes, we can do something like you mentioned during the code generation. For types that include REAL we can skip deriving Eq, Will take a look at it. But for the immediate need the approach mentioned in build.rs might be useful.

gabhijit avatar Nov 08 '23 04:11 gabhijit

Yes, for now I am just deriving PartialEq and I don't think it will cause issues. Frankly, I do not fully understand what the difference is between the Rust compiler-generated functionality when deriving PartialEq vs Eq. It may be minimal, in which case the impact of this issue would also be minimal.

I caught this because previously, I could blindly opt into both eq and partial-eq when calling the code generation script for all of the ASN.1 files I was using, but that isn't the case anymore. And in the event that for whatever reason, someone really needed to derive Eq on a type that did not include a REAL in it, then the inclusion of a REAL anywhere in the spec would effectively poison the entire file.

It is possible that the amount of people who would truly be affected by this in a way that they could not easily work around is near 0.

gth828r avatar Nov 08 '23 05:11 gth828r

I agree with you - trying to handle special cases often ends up being annoying for doing normal things. Also for structs where it is required to impl one of the derive traits, I do this 'after' code generation (and that's possible because it is still same crate). So this doesn't stop people from implementing those traits if at all required or do any non-standard things.

Also: I have not understood the subtle differences between Eq and PartialEq fully yet and often keep confusing.

gabhijit avatar Nov 08 '23 05:11 gabhijit