Skip to content

consts in patterns in const contexts with feature effects causes warning (soon: error) about not implementing PartialEq #119398

Closed
#135064
Closed
consts in patterns in const contexts with feature effects causes warning (soon: error) about not implementing PartialEq#119398
#135064
Labels
A-diagnosticsArea: Messages for errors, warnings, and lintsA-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.E-needs-testCall for participation: An issue has been fixed and does not reproduce, but no test has been added.F-const_trait_impl`#![feature(const_trait_impl)]`F-effects`#![feature(effects)]`T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Description

@bend-n
bend-n
opened on Dec 29, 2023

Code

#![feature(effects)]
#[derive(Eq, PartialEq)]
pub struct Y(u8);
pub const GREEN: Y = Y(4);
pub const fn is_green(x: Y) -> bool {
    match x { GREEN => true, _ => false }
}

Current output

warning: to use a constant of type `Y` in a pattern, the type must implement `PartialEq`
 --> src/main.rs:7:15
  |
7 |     match x { GREEN => true, _ => false }
  |               ^^^^^
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
  = note: `#[warn(const_patterns_without_partial_eq)]` on by default

Desired output

Rationale and extra context

the T is Derive(PartialEq); why does it warn?

Other cases

No response

Anything else?

as requested by @fmease

Activity

  1. bend-n
    added
    A-diagnosticsArea: Messages for errors, warnings, and lints
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    on Dec 29, 2023
  2. rustbot
    added
    needs-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
    on Dec 29, 2023

  3. compiler-errors commented on Dec 29, 2023

    @compiler-errors
    compiler-errors
    on Dec 29, 2023
    Contributor

    the T is Derive(PartialEq); why does it warn?

    Because T does not implement const PartialEq.

  4. bend-n commented on Dec 29, 2023

    @bend-n
    bend-n
    on Dec 29, 2023
    ContributorAuthor

    Yes, i understand that, but it still seems like a incorrect diagnostic.

  5. fmease commented on Dec 29, 2023

    @fmease
    fmease
    on Dec 29, 2023
    Member

    #[derive_const] doesn't help btw., it doesn't get recognized by the lint either (it throws an error).

  6. Noratrieb
    added
    F-effects`#![feature(effects)]`
    and removed
    needs-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
    on Dec 29, 2023

  7. compiler-errors commented on Dec 29, 2023

    @compiler-errors
    compiler-errors
    on Dec 29, 2023
    Contributor

    @fmease: What do you mean by that? That lint isn't recognizing a derive, it's looking for a PartialEq impl.

    For the record, this code works:

    #![feature(derive_const, effects)]

#[derive_const(PartialEq)]
#[derive(Eq)] // `Eq` is not a const trait, so we must regular-derive it.
pub struct Y(u8);
pub const GREEN: Y = Y(4);
pub const fn is_green(x: Y) -> bool {
    match x { GREEN => true, _ => false }
}

  8. bend-n commented on Dec 29, 2023

    @bend-n
    bend-n
    on Dec 29, 2023
    ContributorAuthor

    Er, excuse me, but what is derive_const? the unstable docs just say "this feature is internal to the rust compiler".

  9. fmease commented on Dec 29, 2023

    @fmease
    fmease
    on Dec 29, 2023
    Member

    Revisiting this, yesterday I totally missed that the match was inside a const fn, I just saw fn 🤦. So it definitely makes sense that const_patterns_without_partial_eq expects an impl const.

    Therefore this reduces to a pure diagnostics issue (actually I'm not super sure, see RalfJ's RFC, rust-lang/rfcs#3535). Ideally, we would emit something like:

    - warning: to use a constant of type `Y` in a pattern, the type must implement `PartialEq`
+ warning: to use a constant of type `Y` in a pattern in a const context, the type must implement `const PartialEq`
   --> src/main.rs:7:15
    |
  7 |     match x { GREEN => true, _ => false }
    |               ^^^^^
    |
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
    = note: `#[warn(const_patterns_without_partial_eq)]` on by default

  10. fmease commented on Dec 29, 2023

    @fmease
    fmease
    on Dec 29, 2023
    Member

    the unstable docs just say "this feature is internal to the rust compiler".

    Oh, that's unfortunate, it's an auto-generated message. To clarify, it says likely internal (via). In this case, I don't think it's internal but simply an experimental feature without a tracking issue since the keyword generics / effects story is still ongoing.

    #[derive_const] is similar to #[derive] except that the generated impl becomes an impl const (see feature const_trait_impl for more).

  11. fmease
    added
    A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.
    on Dec 29, 2023
  12. fmease
    changed the title [-]match on const triggers incorrect warning with `effects` feature[/-] [+]confusing lint diagnostic on consts in patterns in const contexts with feature `effects`[/+] on Dec 29, 2023

  13. fmease commented on Dec 29, 2023

    @fmease
    fmease
    on Dec 29, 2023
    Member

    Hmm, nonetheless it's weird that we don't lint against that without effects, for forward compat we should (?), although there wouldn't be a way to fix it on stable. I also don't know how the lint code would be able to check that properly without effects enabled. Again, see rust-lang/rfcs#3535, section Unresolved Questions. By the way, sorry for my rambling and back and forth, it's still morning over here and I'm a tad tired ^^'.

  14. RalfJung
    changed the title [-]confusing lint diagnostic on consts in patterns in const contexts with feature `effects`[/-] [+]consts in patterns in const contexts with feature `effects` causes warning (soon: error) about not implementing PartialEq[/+] on Feb 14, 2024

  15. RalfJung commented on Feb 14, 2024

    @RalfJung
    RalfJung
    on Feb 14, 2024
    Member

    The lint will become a hard error when #120805 lands.

    Note that this code works on stable and has worked for quite a while:

    #[derive(Eq, PartialEq)]
pub struct TypeThatIsPartialEq(u8);
pub const GREEN: TypeThatIsPartialEq = TypeThatIsPartialEq(4);

pub const fn is_green(x: TypeThatIsPartialEq) -> bool {
    match x { GREEN => true, _ => false }
}

    So, I think there's a bug with the effects feature here somewhere, in how it interacts with the const-pattern check, which leads to the code being rejected with feature(effects).

  16. fmease
    added
    F-const_trait_impl`#![feature(const_trait_impl)]`
    F-effects`#![feature(effects)]`
    and removed
    F-effects`#![feature(effects)]`
    on Nov 15, 2024

  17. fmease commented on Nov 16, 2024

    @fmease
    fmease
    on Nov 16, 2024
    Member

    No longer reproduces.

  18. fmease
    added
    E-needs-testCall for participation: An issue has been fixed and does not reproduce, but no test has been added.
    on Nov 16, 2024

  19. RalfJung commented on Nov 16, 2024

    @RalfJung
    RalfJung
    on Nov 16, 2024
    Member

    Cc @compiler-errors who changed a bunch of stuff here recently.
    Not sure if there was a proper fix or the issue is just masked for now.

  20. compiler-errors commented on Jan 3, 2025

    @compiler-errors
    compiler-errors
    on Jan 3, 2025
    Contributor

    @RalfJung: This is a real fix, because we no longer erroneously require that T: const PartialEq hold for it to be used in a pattern in a const function.

    It's still somewhat conceptually dissatisfying/strange, though. In an abstract sense, it kinda feels right that we enforce that T: const PartialEq hold if it's used in a pattern in a const function, since it seems antithetical to the whole concept of matching being equivalent to emitting PartialEq calls if we don't?

  21. RalfJung commented on Jan 3, 2025

    @RalfJung
    RalfJung
    on Jan 3, 2025
    Member

    In an abstract sense, it kinda feels right that we enforce that T: const PartialEq hold if it's used in a pattern in a const function, since it seems antithetical to the whole concept of matching being equivalent to emitting PartialEq calls if we don't?

    Agreed. But it'd be a breaking change to do that now... so this is basically just a back-compat hack.

  22. matthiaskrgr
    added a commit that references this issue on Jan 4, 2025

    Rollup merge of rust-lang#135064 - RalfJung:const-in-pat-partial-eq-n…

    966a5be
  23. bors
    closed this as completedin 966a5beon Jan 4, 2025
  24. rust-timer
    added a commit that references this issue on Jan 4, 2025

    Unrolled build for rust-lang#135064

    b7a0c81
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-diagnosticsArea: Messages for errors, warnings, and lintsA-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.E-needs-testCall for participation: An issue has been fixed and does not reproduce, but no test has been added.F-const_trait_impl`#![feature(const_trait_impl)]`F-effects`#![feature(effects)]`T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions