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

New lints: Changes to enum variant discriminants #898

Closed
3 tasks done
obi1kenobi opened this issue Sep 1, 2024 · 4 comments · Fixed by #1037
Closed
3 tasks done

New lints: Changes to enum variant discriminants #898

obi1kenobi opened this issue Sep 1, 2024 · 4 comments · Fixed by #1037
Labels
A-lint Area: new or existing lint E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: Mentorship is available for this issue.

Comments

@obi1kenobi
Copy link
Owner

obi1kenobi commented Sep 1, 2024

Background on discriminants: https://doc.rust-lang.org/reference/items/enumerations.html#discriminants

We're looking for three lints total:

  • a public API enum without an explicit repr() has a variant that used to have a well-defined discriminant, but no longer does:
    • We only check the "without explicit repr()" case here. This is because if the enum has a repr(), then its variants always have a well-defined discriminant so there's nothing to lint. The case when the repr() is removed (and therefore might un-define some discriminants) is handled by another lint.
  • a public API enum's variant had its discriminant has changed from its previous value: @dmatos2012

We distinguish between the repr/no-repr cases so that we can offer better error messages. When an enum has an explicit repr, that imposes an ABI (application binary interface) constraint in addition to the API constraint. Without a repr, the numeric value of the discriminant is fixed but its binary representation is not and could be anything.

For examples of querying the repr properly, please look at the enum_repr_int_changed lint. Make sure to include test cases where the repr is #[repr(C, i64)] or #[repr(u8, C)] since both variants are allowed too. The lints must appropriately detect the integer repr in both these cases and more straightforward ones like #[repr(i8)].

@obi1kenobi obi1kenobi added A-lint Area: new or existing lint E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: Mentorship is available for this issue. labels Sep 1, 2024
@dmatos2012
Copy link
Contributor

Ill take the the 2nd bullet point, a public API enum's variant had its discriminant has changed from its previous value lint, which will cover the two bullet points you mention :)

@obi1kenobi
Copy link
Owner Author

Cool, got you marked as owning that. Are you still working on #885 as well, or is that blocked on something?

@dmatos2012
Copy link
Contributor

Cool, got you marked as owning that. Are you still working on #885 as well, or is that blocked on something?

answered

@obi1kenobi
Copy link
Owner Author

Ah @dmatos2012 we actually want four lints for this issue, not two. The second bullet point describes two lints, since we want to lint for "you're breaking ABI" (such as for FFI use cases) separately from "you are breaking API." That will let us make stronger statements than "may break FFI", since hedging is generally bad when reporting errors.

obi1kenobi added a commit that referenced this issue Dec 11, 2024
obi1kenobi added a commit that referenced this issue Dec 11, 2024
obi1kenobi added a commit that referenced this issue Dec 11, 2024
- `enum_discriminants_undefined_non_exhaustive_variant` checks for enums that gain a new non-exhaustive variant, which then causes the discriminant to become undefined.
- `enum_discriminants_undefined_non_unit_variant` checks for enums that gain a new non-unit variant, which then causes the discriminant to become undefined.

Resolves #898.
obi1kenobi added a commit that referenced this issue Dec 11, 2024
- `enum_discriminants_undefined_non_exhaustive_variant` checks for enums that gain a new non-exhaustive variant, which then causes the discriminant to become undefined.
- `enum_discriminants_undefined_non_unit_variant` checks for enums that gain a new non-unit variant, which then causes the discriminant to become undefined.

Resolves #898.
obi1kenobi added a commit that referenced this issue Dec 11, 2024
…d. (#1037)

- `enum_discriminants_undefined_non_exhaustive_variant` checks for enums
that gain a new non-exhaustive variant, which then causes the
discriminant to become undefined.
- `enum_discriminants_undefined_non_unit_variant` checks for enums that
gain a new non-unit variant, which then causes the discriminant to
become undefined.

Resolves #898.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: new or existing lint E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: Mentorship is available for this issue.
Projects
None yet
2 participants