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

Surprising diagnostics about using const value in match pattern #92454

Open
djc opened this issue Dec 31, 2021 · 6 comments
Open

Surprising diagnostics about using const value in match pattern #92454

djc opened this issue Dec 31, 2021 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@djc
Copy link
Contributor

djc commented Dec 31, 2021

Given the following code (playground:

use std::borrow::Cow;

fn main() {
    let foo = Foo { inner: "foo".into() };
    println!("{}", match foo {
        FOO => "match",
        _ => "no"
    });
}

#[derive(PartialEq, Eq)]
struct Foo {
    inner: Cow<'static, str>,
}

const FOO: Foo = Foo { inner: Cow::Borrowed("foo") };

The current output is:

error: to use a constant of type `Cow` in a pattern, `Cow` must be annotated with `#[derive(PartialEq, Eq)]`
 --> src/main.rs:6:9
  |
6 |         FOO => "match",
  |         ^^^

Surprisingly to me at least, using a custom PartialEq implementation still results in the same issue (playground):

struct Foo {
    inner: Cow<'static, str>,
}

impl PartialEq for Foo {
    fn eq(&self, other: &Self) -> bool {
        self.inner.as_ref() == other.inner.as_ref()
    }
}

impl Eq for Foo {}

This seems pretty unclear:

  • Why is the compiler telling me I should derive PartialEq for Cow when I'm comparing matching against Foo?
  • Why is the compiler telling me to derive PartialEq instead of having a custom impl?

I don't know exactly what kind of contract match depends on, but I would have thought any kind of PartialEq impl (that is, even non-const) would be enough here.

Original issue here: open-telemetry/opentelemetry-rust#685. This also links to a potential fix in sfackler/rust-postgres@05a0643 (involving adding another layer of indirection), but I'd really like to understand why that is even necessary.

(To be clear: the diagnostic is unclear here, but maybe there's an actual compiler problem here, too?)

cc @estebank

@djc djc added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 31, 2021
@fee1-dead
Copy link
Member

fee1-dead commented Dec 31, 2021

This is because Cow is not "StructualEq" (because it compares the borrowed version only and it is possible for Cow::Owned to be equal to a Cow::Borrowed) so constants of that type cannot be used in matches. I do believe the diagnostic should be improved.

cc @rust-lang/wg-const-eval

@djc
Copy link
Contributor Author

djc commented Dec 31, 2021

@fee1-dead that's helpful, my intuition was that it had something to do with const stuff.

Still, it seems this story is still really hard to explain:

  • So the type actually has to be StructuralEq?
  • And you can only be StructuralEq if you derive Eq?
  • But if I derive Eq and keep my custom PartialEq, that's not sufficient either

This all seems really weird because apparently StructuralEq is not just a trait implemented by a type but also something that must be present for all of its inner types... or something?

@RalfJung
Copy link
Member

RalfJung commented Jan 1, 2022

StructuralEq is kind of a mess currently and something we want to clean up. Also see #74446.

@djc
Copy link
Contributor Author

djc commented Jan 1, 2022

Okay, but then we should definitely improve diagnostics such that the messiness from an unstable filter leak out into stable Rust via crappy errors (especially as there seems to be no clear/fast path towards stabilization).

@raphaelcohn
Copy link

Just took an hour googling to end up here with the same error message. Anything that at least adds a bit more information (eg 'can not be structurally equal' or includes a compiler error number) would be much appreciated.

@RalfJung
Copy link
Member

I don't know exactly what kind of contract match depends on, but I would have thought any kind of PartialEq impl (that is, even non-const) would be enough here.

The error explicitly says #[derive(PartialEq, Eq)]. So it doesn't explain why, but it does explain that your assumption here is wrong.

See rust-lang/lang-team#220 for some recent discussion of the subject. The current plan is to keep this a hard error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants