ystero-dev / hampi

Rust ASN.1 Toolkit
Other
44 stars 17 forks source link

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

Open gth828r opened 11 months ago

gth828r commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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.

gabhijit commented 11 months ago

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.

gth828r commented 11 months ago

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.

gabhijit commented 11 months ago

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.