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

Add #[non_exhaustive] option to enum codegen options #43

Open
danburkert opened this issue Oct 14, 2017 · 4 comments
Open

Add #[non_exhaustive] option to enum codegen options #43

danburkert opened this issue Oct 14, 2017 · 4 comments

Comments

@danburkert
Copy link
Collaborator

It would be nice to be able to have a way to tag generated enums with #[non_exhaustive]. The feature is still in development, but once it's implemented and unstable, prost can in theory take advantage of it without requiring nightly.

@per-gron
Copy link

per-gron commented Jul 5, 2018

Is the plan to just add #[non_exhaustive] to the generated enums as they are today and keep the i32 storage, or to only have the enum directly in the generated structs?

It would be nice to use enums directly but the RFC says things that imply that it wouldn't be safe to do so: "Outside the crate that defines the enum, users should be required to add a wildcard arm to ensure forward-compatibility, like so: [match expression with default arm] And it should not be marked as dead code, even if the compiler does mark it as dead and remove it."

@per-gron
Copy link

per-gron commented Jul 5, 2018

I think I just found the answer to my own question in rust-lang/rust#36927 (comment) : even with #[non_exhaustive] it is wrong to ever have an enum with a value that is not specified in code so #[non_exhaustive] cannot be used to replace the underlying int storage.

@vlisivka
Copy link

#[non_exhaustive] is stabilized. Can we switch back to enum's now, please?

@danburkert
Copy link
Collaborator Author

To be clear, this issue is not about changing the representation of enum message fields. The current strategy of using i32s is here to stay. This issue is about applying the #[non_exhaustive] tag to the generated Rust enum types, and only on an opt-in basis via code gen builder arguments.

wchargin added a commit to wchargin/prost that referenced this issue Nov 7, 2020
Builds are failing on unrelated pull requests (e.g., tokio-rs#326) because the
`curl` crate has pushed a new version that depends on Rust 1.40, while
our CI tests against 1.39. We could probably bump to require 1.40 as
well, especially since we want to use the same `#[non_exhaustive]`
attribute, per tokio-rs#43. But a minimal change to unblock development and keep
the current minimum stable Rust version is to just pin `curl < 0.4.34`.

Test Plan:
Running `cargo +1.39.0 test --workspace --all-targets` passes on my
Linux machine.

wchargin-branch: deps-curl-pre-0.4.34
wchargin-source: c2910530bd49190605176c9f69f3aff1863443c2
danburkert pushed a commit that referenced this issue Nov 8, 2020
Builds are failing on unrelated pull requests (e.g., #326) because the
`curl` crate has pushed a new version that depends on Rust 1.40, while
our CI tests against 1.39. We could probably bump to require 1.40 as
well, especially since we want to use the same `#[non_exhaustive]`
attribute, per #43. But a minimal change to unblock development and keep
the current minimum stable Rust version is to just pin `curl < 0.4.34`.

Test Plan:
Running `cargo +1.39.0 test --workspace --all-targets` passes on my
Linux machine.

wchargin-branch: deps-curl-pre-0.4.34
wchargin-source: c2910530bd49190605176c9f69f3aff1863443c2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants