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 diagnostics for specific cases for const/type mismatch err #82055

Merged
merged 3 commits into from
Feb 18, 2021

Conversation

JulianKnodt
Copy link
Contributor

For now, this adds at least more information so better diagnostics can be emitted for const mismatch errors.

I'm not sure what exactly we want to emit, so I've left notes there temporarily, also to see if this is the right approach

r? @lcnr
cc: @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 13, 2021
@rust-log-analyzer

This comment has been minimized.

@JulianKnodt JulianKnodt force-pushed the ty_where_const branch 2 times, most recently from 7051772 to ce12661 Compare February 13, 2021 17:01
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Thank you for tackling this! It'll make this feature so much nicer to use :)

compiler/rustc_typeck/src/astconv/generics.rs Outdated Show resolved Hide resolved
Comment on lines 10 to 14
error[E0747]: type provided when a constant was expected
--> $DIR/diagnostics.rs:7:16
|
LL | impl Foo for A<N> {}
| ^
Copy link
Contributor

Choose a reason for hiding this comment

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

This one should probably be silenced when the one above is emitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how to detect when the above is emitted, as from the path resolution alone I cannot tell, but I've changed the message to be more pertinent.

Comment on lines +7 to +8
LL | impl Foo for A<N> {}
| ^ help: a struct with a similar name exists: `A`
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally this would suggest "you meant to add a const N here and write this as A<{ N }>".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's hard to tell if a Res::Err is this case or the other case where the path was not given enough generic args

src/test/ui/const-generics/diagnostics.stderr Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Left a few nitpicks, we're close to landing this I think :)

compiler/rustc_typeck/src/astconv/generics.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/generics.rs Outdated Show resolved Hide resolved
@JulianKnodt
Copy link
Contributor Author

Just a note, it's currently implemented where it will also emit the ordering help just for demonstration purposes, should I take it out?

@rust-log-analyzer

This comment has been minimized.

@estebank
Copy link
Contributor

Just a note, it's currently implemented where it will also emit the ordering help just for demonstration purposes, should I take it out?

Ideally we would not emit that suggestion, but I guess it is fine for now and can be dealt with in a separate PR :-/

@bors r+

@bors
Copy link
Contributor

bors commented Feb 16, 2021

📌 Commit b97951b has been approved by estebank

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 16, 2021
@estebank
Copy link
Contributor

r? @estebank

@rust-highfive rust-highfive assigned estebank and unassigned lcnr Feb 16, 2021
@estebank
Copy link
Contributor

@JulianKnodt could you file tickets for the two outstanding issues (silencing the reorder suggestion and handle the impl Foo for Bar<N>) linking to the right comments in this PR?

@JulianKnodt
Copy link
Contributor Author

ah sure, but why shouldn't I just silence the reordering here since I know where to put it?

@estebank
Copy link
Contributor

Oh, if you can, go ahead.

@estebank
Copy link
Contributor

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 16, 2021
@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 16, 2021

📌 Commit f520295 has been approved by estebank

@bors 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 Feb 16, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 16, 2021
Add diagnostics for specific cases for const/type mismatch err

For now, this adds at least more information so better diagnostics can be emitted for const mismatch errors.

I'm not sure what exactly we want to emit, so I've left notes there temporarily, also to see if this is the right approach

r? `@lcnr`
cc: `@estebank`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 17, 2021
Add diagnostics for specific cases for const/type mismatch err

For now, this adds at least more information so better diagnostics can be emitted for const mismatch errors.

I'm not sure what exactly we want to emit, so I've left notes there temporarily, also to see if this is the right approach

r? ``@lcnr``
cc: ``@estebank``
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#82055 (Add diagnostics for specific cases for const/type mismatch err)
 - rust-lang#82155 (Use !Sync std::lazy::OnceCell in usefulness checking)
 - rust-lang#82202 (add specs for riscv32/riscv64 musl targets)
 - rust-lang#82203 (Move some tests to more reasonable directories - 4)
 - rust-lang#82211 (make `suggest_setup` help messages better)
 - rust-lang#82212 (Remove redundant rustc_data_structures path component)
 - rust-lang#82240 (remove useless ?s (clippy::needless_question_marks))
 - rust-lang#82243 (Add more intra-doc links to std::io)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0c25d15 into rust-lang:master Feb 18, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 18, 2021
@lcnr lcnr added the A-const-generics Area: const generics (parameters and arguments) label Dec 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants