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 context to "const in pattern" errors #133233

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

estebank
Copy link
Contributor

Each commit addresses specific diagnostics.

  • Add primary span labels
  • Point at const item, and const generic param definition
  • Reword messages and notes
  • Point at generic param through which an associated const is being referenced
  • Silence const in pattern with evaluation errors when they come from const items that already emit a diagnostic
  • On non-structural type in const used as pattern, point at the type that should derive PartialEq

@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2024

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 20, 2024
@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2024

Some changes occurred in match checking

cc @Nadrieril

@rust-log-analyzer

This comment has been minimized.

@bors

This comment was marked as resolved.

Centralize emitting an error in `const_to_pat` so that all errors from that evaluating a `const` in a pattern can add addditional information. With this, now point at the `const` item's definition:

```
error[E0158]: constant pattern depends on a generic parameter
  --> $DIR/associated-const-type-parameter-pattern.rs:20:9
   |
LL | pub trait Foo {
   | -------------
LL |     const X: EFoo;
   |     ------------- constant defined here
...
LL |         A::X => println!("A::X"),
   |         ^^^^
```
Silence errors that are implied by the errors in the `const` item definition.

Add a primary span label.
Conform to error style guide.
```
error[E0158]: constant pattern depends on a generic parameter, which is not allowed
  --> $DIR/associated-const-type-parameter-pattern.rs:20:9
   |
LL | pub trait Foo {
   | -------------
LL |     const X: EFoo;
   |     ------------- constant defined here
...
LL | pub fn test<A: Foo, B: Foo>(arg: EFoo) {
   |             - constant depends on this generic param
LL |     match arg {
LL |         A::X => println!("A::X"),
   |         ^^^^ `const` depends on a generic parameter
```
- Add primary span labels.
- Point at const generic parameter used as pattern.
- Point at statics used as pattern.
- Point at let bindings used in const pattern.
```
error: trait object `dyn Send` cannot be used in patterns
  --> $DIR/issue-70972-dyn-trait.rs:6:9
   |
LL | const F: &'static dyn Send = &7u32;
   | -------------------------- constant defined here
...
LL |         F => panic!(),
   |         ^ trait object can't be used in patterns
```
- Point at type that should derive `PartialEq` to be structural.
- Point at manual `impl PartialEq`, explaining that it is not sufficient to be structural.

```
error: constant of non-structural type `MyType` in a pattern
  --> $DIR/const-partial_eq-fallback-ice.rs:14:12
   |
LL | struct MyType;
   | ------------- `MyType` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
...
LL | const CONSTANT: &&MyType = &&MyType;
   | ------------------------ constant defined here
...
LL |     if let CONSTANT = &&MyType {
   |            ^^^^^^^^ constant of non-structural type
   |
note: the `PartialEq` trait must be derived, manual `impl`s are not sufficient; see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
  --> $DIR/const-partial_eq-fallback-ice.rs:5:1
   |
LL | impl PartialEq<usize> for MyType {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
Unify wording with the regular non-structural type error.
@rust-log-analyzer

This comment has been minimized.

…e" error

Point at types that need to be marked with `#[derive(PartialEq)]`.

We use a visitor to look at a type that isn't structural, looking for all ADTs that don't derive `PartialEq`. These can either be manual `impl PartialEq`s or no `impl` at all, so we differentiate between those two cases to provide more context to the user. We also only point at types and impls from the local crate, otherwise show only a note.

```
error: constant of non-structural type `&[B]` in a pattern
  --> $DIR/issue-61188-match-slice-forbidden-without-eq.rs:15:9
   |
LL | struct B(i32);
   | -------- must be annotated with `#[derive(PartialEq)]` to be usable in patterns
LL |
LL | const A: &[B] = &[];
   | ------------- constant defined here
...
LL |         A => (),
   |         ^ constant of non-structural type
   |
   = note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
```
@Nadrieril
Copy link
Member

I can take this

r? @Nadrieril

@rustbot rustbot assigned Nadrieril and unassigned nnethercote Nov 21, 2024
@estebank
Copy link
Contributor Author

@Nadrieril review best done per commit. Feel free to say "I've done these N commits, split off the rest". I just felt it was useful to see the end result of all changes on the diagnostics in aggregate, but they don't need to land together, just in order.


/// Given a type with type parameters, visit every ADT looking for types that need to
/// `#[derive(PartialEq)]` to be a structural type.
fn extend_type_not_partial_eq<'tcx>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This could also be used for the non-fallthrough case (where the top-level type isn't structural), as the visitor would work for that too.
  • We could rework this to not operate on the Diag directly, instead create a new Subdiagnostic type that gets added to the Diagnostic as a Vec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants