Clarify the 'use a constant in a pattern' error message by jamen · Pull Request #108548 · rust-lang/rust

jamen · 2023-02-28T00:46:03Z

use std::borrow::Cow;

const ERROR_CODE: Cow<'_, str> = Cow::Borrowed("23505");

fn main() {
    let x = Cow::from("23505");
    
    match x {
        ERROR_CODE => {}
    }
}

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

error: could not compile `playground` due to previous error

It seems helpful to link to StructuralEq in this message. I was a little confused, because Cow<'_, str> implements PartialEq and Eq, but they're not derived, which I learned is necessary for structural equality and using constants in patterns (thanks to the Rust community Discord server)

For tests, should I update every occurrence of this message? I see tests where this is still a warning and I'm not sure if I should update those.

rustbot · 2023-02-28T00:46:11Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (or someone else) soon.

Please see the contribution instructions for more information.

compiler-errors · 2023-02-28T04:09:54Z

I don't think that this error message should mention StructuralEq, but regardless, it should be done as a separate "note:" section on the diagnostic, and not in the primary message of the error.

jamen · 2023-02-28T05:01:53Z

Ok I attempted that

petrochenkov · 2023-02-28T10:03:38Z

CI is failing.
x.py test --bless tests/ui should update the affected test outputs.
@rustbot author

jamen · 2023-02-28T21:13:48Z

fixed an accidental edit. tests unaffected

petrochenkov · 2023-03-01T08:00:12Z

@rustbot ready
r? @compiler-errors

jamen · 2023-03-02T20:07:29Z

I don't think that this error message should mention StructuralEq

Why not? To me the message explains the what but not the why. Seems like a helpful piece of information.

compiler-errors · 2023-03-07T05:47:05Z

My only concern is I'm not sure if it's conventional to document never-stable, compiler implementation details in user-facing (non-nightly-feature-related) error messages...

Maybe someone else from @rust-lang/wg-diagnostics can help make a call here.

This also could really use a test demonstrating the note. otherwise r=me on the implementation though.

oli-obk · 2023-03-07T07:47:54Z

I think we could mention StructualEq as "extended reading material", but in order to fix your issue, the more important thing to me is to have a note saying "the traits must be derived, manual impls are not sufficient" first.

bors · 2023-03-12T07:31:49Z

☔ The latest upstream changes (presumably #108682) made this pull request unmergeable. Please resolve the merge conflicts.

compiler-errors · 2023-03-14T03:54:13Z

This has a conflict

Would appreciate an additional note (maybe above the one you added) that mentions that manual impls of PartialEq and Eq are not sufficient

compiler-errors · 2023-03-25T07:05:27Z

compiler/rustc_mir_build/messages.ftl

please make this into a note, not the primary message. same below.

also, why did you take away the link to Structural{Partial}Eq? That was fine, as long as it was paired with an additional note that explains what those traits means in practice.

estebank · 2023-03-25T18:05:04Z

Can we also make sure that there's a test exercising this message?

jamen · 2023-03-26T23:58:00Z

I see tests in tests/ui/consts/const_in_pattern but they're unaffected by adding notes (they are affected when changing the main message). How do I go about that?

compiler-errors · 2023-03-26T23:59:02Z

You need to give the second note another name and declare it in the diagnostic struct -- the first is being overwritten. Fluent file slug names are not deduplicated.

rustbot · 2023-03-27T15:08:48Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

jamen · 2023-03-27T20:39:31Z

src/tools/clippy/tests/ui/crashes/ice-6254.stderr

   = note: for more information, see issue #62411 <https://github.com/rust-lang/rust/issues/62411>
+   = note: the traits must be derived, manual `impl`s are not sufficient
+   = note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralEq.html for details
   = note: `-D indirect-structural-match` implied by `-D warnings`


unsure why this appears in the middle of these other notes

I don't think it really matters.

compiler-errors · 2023-03-27T23:36:31Z

@bors r+ rollup

bors · 2023-03-27T23:36:34Z

📌 Commit 73c34cb has been approved by compiler-errors

It is now in the queue for this repository.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#108548 (Clarify the 'use a constant in a pattern' error message) - rust-lang#109565 (Improve documentation for E0223) - rust-lang#109661 (Fix LVI test post LLVM 16 update) - rust-lang#109667 (Always set `RUSTC_BOOTSTRAP` with `x doc`) - rust-lang#109669 (Update books) - rust-lang#109678 (Don't shadow the `dep_node` var in `incremental_verify_ich_failed`) - rust-lang#109682 (Add `#[inline]` to CStr trait implementations) - rust-lang#109685 (Make doc comment a little bit more accurate) - rust-lang#109687 (Document the heuristics IsTerminal uses on Windows) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Clarify the 'use a constant in a pattern' error message ```rs use std::borrow::Cow; const ERROR_CODE: Cow<'_, str> = Cow::Borrowed("23505"); fn main() { let x = Cow::from("23505"); match x { ERROR_CODE => {} } } ``` ``` error: to use a constant of type `Cow` in a pattern, `Cow` must be annotated with `#[derive(PartialEq, Eq)]` --> src/main.rs:9:9 | 9 | ERROR_CODE => {} | ^^^^^^^^^^ error: could not compile `playground` due to previous error ``` It seems helpful to link to StructuralEq in this message. I was a little confused, because `Cow<'_, str>` implements PartialEq and Eq, but they're not derived, which I learned is necessary for structural equality and using constants in patterns (thanks to the Rust community Discord server) For tests, should I update every occurrence of this message? I see tests where this is still a warning and I'm not sure if I should update those.

rustbot assigned petrochenkov Feb 28, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 28, 2023

jamen marked this pull request as draft February 28, 2023 00:48

petrochenkov marked this pull request as ready for review February 28, 2023 10:02

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 28, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 1, 2023

rustbot assigned compiler-errors and unassigned petrochenkov Mar 1, 2023

compiler-errors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 14, 2023

compiler-errors reviewed Mar 25, 2023

View reviewed changes

Add notes to non-structural const in pattern error message

73c34cb

jamen commented Mar 27, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 27, 2023

matthiaskrgr mentioned this pull request Mar 28, 2023

Rollup of 9 pull requests #109690

Merged

bors merged commit a7c07cf into rust-lang:master Mar 28, 2023

rustbot added this to the 1.70.0 milestone Mar 28, 2023

Uh oh!

Conversation

jamen commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 28, 2023

Uh oh!

compiler-errors commented Feb 28, 2023

Uh oh!

jamen commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Feb 28, 2023

Uh oh!

jamen commented Feb 28, 2023

Uh oh!

petrochenkov commented Mar 1, 2023

Uh oh!

jamen commented Mar 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

compiler-errors commented Mar 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oli-obk commented Mar 7, 2023

Uh oh!

bors commented Mar 12, 2023

Uh oh!

compiler-errors commented Mar 14, 2023

Uh oh!

compiler-errors Mar 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estebank commented Mar 25, 2023

Uh oh!

jamen commented Mar 26, 2023

Uh oh!

compiler-errors commented Mar 26, 2023

Uh oh!

rustbot commented Mar 27, 2023

Uh oh!

jamen Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

compiler-errors Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Mar 27, 2023

Uh oh!

bors commented Mar 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jamen commented Feb 28, 2023 •

edited

Loading

jamen commented Feb 28, 2023 •

edited

Loading

jamen commented Mar 2, 2023 •

edited

Loading

compiler-errors commented Mar 7, 2023 •

edited

Loading

compiler-errors Mar 25, 2023 •

edited

Loading