Don’t declare schema for empty enum variants #227
Conversation
| <ASalad as borsh::BorshSchema>::add_definitions_recursively(definitions); | ||
| <ASausage as borsh::BorshSchema>::add_definitions_recursively(definitions); | ||
| let definition = borsh::schema::Definition::Enum { | ||
| tag_width: 1, | ||
| variants: borsh::__private::maybestd::vec
There was a problem hiding this comment.
actually, this change implies, that "nil" starts to mean different things in different contexts (as if it were mapped to 2 different Definition-s, picked from in different contexts).
It adds a new rule for interpreting "nil" as a unit struct variant in these situations.
Let's keep it more verbose (and more straightforward) as it is now. One looks at a variant definition, and sees that it's a unit struct.
There was a problem hiding this comment.
But with this change one can look at a variant definition and sees it’s a unit struct. In fact, it’s now easier to see it’s a unit struct since when I see ("Bacon", "nil") I immediately know it’s a variant with no data; when I see ("Bacon", "ABecon") I know have to look up what ABecon is.
What’s more, nil can be mapped to an empty tuple which would better match that it’s used for (). (To be honest, I’d actually change the declaration from "nil" to "()" and then also change declaration for tuples to use (...)).
There was a problem hiding this comment.
No information of the source type is lost by defining empty enum variants as unit type rather than creating a custom declaration for them. At the same time, not doing that simplifies the schemas and reduces chances for name collisions in derived schemas.
No information of the source type is lost by defining empty enum variants
as unit type rather than creating a custom declaration for them. At the
same time, not doing that simplifies the schemas and reduces chances for
name collisions in derived schemas.