Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does adding an enum variant break backwards compat? #16

Closed
SUPERCILEX opened this issue Jan 13, 2024 · 10 comments
Closed

Does adding an enum variant break backwards compat? #16

SUPERCILEX opened this issue Jan 13, 2024 · 10 comments

Comments

@SUPERCILEX
Copy link

I just want to confirm: anything written as type A will be incompatible for reading as if it were type A'? Specifically for enum variants.

@finnbear
Copy link
Member

finnbear commented Jan 13, 2024

Hello,

If you use the serde version (Serialize, Deserialize), then it's ok to add new variants at the end of your enum. Since bitcode can't know the total number of variants, they might as well have been there all along. In general, A and A' are compatible if what serde exposes is compatible.

If you use the derive version (Encode, Decode), which does know exactly which variants exist, then there is a high chance (and you must assume) that compatibility is broken. As a reward, enum's (among other things) take fewer bits than with serde.

@SUPERCILEX
Copy link
Author

That's great to know, thanks! Can this (and anything else compatibility related) be documented? Happy to close this issue, but it'd be nice to have this stuff in the readme and crate docs.

@SUPERCILEX
Copy link
Author

Actually, can I turn this into a feature request? I'd like to be able to specify the number of bits to reserve for an enum so I can choose how many I want to have for future compat. Really the setting should be "number of available variants" and in the backend it rounds up to a power of two or does something smarter.

@finnbear
Copy link
Member

finnbear commented Jan 13, 2024

Can this (and anything else compatibility related) be documented?

This is not a "feature" we want to advertise. bitcode is not intended for compatibility when changing types or versions. If and when serde allows knowing the total number of enum variants, we'll optimize bitcode and remove this "feature."

(the existence of this issue - even after it is closed - serves as a small amount of searchable documentation, which is probably enough)

Actually, can I turn this into a feature request?

Unfortunately the ability to reserve enum variants is contingent on a few assumptions about how bitcode would organize data. A future version of bitcode might need to know the size/alignment/etc. of all variants or the enum itself, which wouldn't be known when reserving variants. We have an internal project that completely changes those assumptions and, while we might release it under a different name, we don't want to overconstrain bitcode's implementation either.

@SUPERCILEX
Copy link
Author

bitcode is not intended for compatibility when changing types or versions

This is what should be documented then. Also when you say versions, you mean major versions right?

@finnbear
Copy link
Member

finnbear commented Jan 13, 2024

Also when you say versions, you mean major versions right?

Yes, e.g. 0.4.0 to 0.5.0 or 1.0.0 to 2.0.0. Not minor/patch changes.

@finnbear
Copy link
Member

finnbear commented Jan 13, 2024

Also, I talked with @caibear and another workaround (for Encode and Decode) is to add some placeholder variants for future use:

#[derive(Encode, Decode)]
enum Extensible {
    A,
    B(String),
    #[doc(hidden)]
    Placeholder1,
    #[doc(hidden)]
    Placeholder2,
}

Then, as long as you never use the placeholders, you can change their definition to be a tuple or struct variant.

@SUPERCILEX
Copy link
Author

Clever!

can change their definition to be a tuple or struct variant

Is this guaranteed to always work? If so, that's good enough for me.

@finnbear
Copy link
Member

finnbear commented Jan 13, 2024

Is this guaranteed to always work? If so, that's good enough for me.

Probably, unless we need to deprecate it in a major version (in which case you would have to break compatibility anyway by upgrading if you needed to upgrade).

@SUPERCILEX
Copy link
Author

That's fine, thanks!

@caibear caibear closed this as completed Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants