Don't derive Eq for types that contain REAL - #145
Merged
Merged
Conversation
gabhijit
previously approved these changes
Sep 23, 2026
gabhijit
left a comment
gabhijit
left a comment
Collaborator
There was a problem hiding this comment.
The PR looks good for a couple of comments that I have added. Better to warn! for the types where the Eq derive was skipped, when the user has asked for it.
Also, it might be a good idea to squash these into fewer related commits.
added 2 commits
September 23, 2026 16:37
f64 is only PartialEq, so deriving Eq on a type that has a REAL in it (directly, inside a SEQUENCE / CHOICE / SEQUENCE OF / open type, or through references to other such types) breaks the generated code. Collect those types before generating and leave Eq out for them. A warning is logged for each type where Eq was asked for but skipped. Fixes ystero-dev#110 Signed-off-by: rizwan alam <akaify@cdot.in>
Signed-off-by: rizwan alam <akaify@cdot.in>
rizwan3659
force-pushed
the
skip-eq-for-real
branch
from
September 23, 2026 11:07
a451e7a to
1a971ea
Compare
Author
|
@gabhijit Thanks for the review! Addressed both comments:
|
gabhijit
approved these changes
Sep 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
REAL is generated as
f64, which is onlyPartialEq. Right now the same derive list goes on every type, so asking forEqbreaks the build for any spec that has a REAL in it (E2SM-KPM, for example).This change collects all types that contain a REAL before generating code: directly, inside a SEQUENCE / CHOICE / SEQUENCE OF / open type, or through a reference to another such type. It keeps going over the types until nothing new is found, so chains of references are handled too.
Eqis left out only for those types. Everything else still gets it, so specs without REAL generate the same code as before.I added a test that compiles a small module with
Eqenabled and checks the derives on each generated type.Fixes #110