Skip to content

Unify E0243, E0244 and E0107 #53525

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

Closed
varkor opened this issue Aug 20, 2018 · 14 comments
Closed

Unify E0243, E0244 and E0107 #53525

varkor opened this issue Aug 20, 2018 · 14 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@varkor
Copy link
Member

varkor commented Aug 20, 2018

E0243 and E0244 are both errors about providing the wrong number of generic arguments. Having both is unnecessary: it'd be better if they were unified with E0107 — the analogous error for lifetimes.

GenericArgMismatchErrorCode can go once this is done.

@varkor varkor added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. C-cleanup Category: PRs that clean code up or issues documenting cleanup. labels Aug 20, 2018
@varkor varkor changed the title Unify E0243 and E0244 Unify E0243, E0244 and E0107 Aug 20, 2018
@iancormac84
Copy link
Contributor

Minor nit: The link for E0107 actually points to E0243.

@varkor
Copy link
Member Author

varkor commented Aug 20, 2018

Fixed, thanks!

@varkor varkor added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Aug 20, 2018
@matthew-russo
Copy link
Contributor

Can I take this? Would be my first contribution so might take me a few days.

@varkor
Copy link
Member Author

varkor commented Aug 20, 2018

@mcr431: absolutely! The diagnostics are in src/librustc_typeck/diagnostics.rs. You'll want to add: #### Note: this error code is no longer emitted by the compiler. for E0243 and E0244 and change the text for E0107 to refer to all generic arguments (you can probably merge the examples from E0243 and E0244 into its examples).

You can then remove GenericArgMismatchErrorCode here:

// FIXME(#53525): these error codes should all be unified.
struct GenericArgMismatchErrorCode {
lifetimes: (&'static str, &'static str),
types: (&'static str, &'static str),
}

and just use E0107 instead (you might need to do a little cleaning up in check_generic_arg_count). Then you can run all the ui tests with --bless to update them to the new error codes (./x.py test src/test/ui -i --stage 1 --bless). That should be it!

If you have any questions, feel free to ask here or on Discord / #rustc. Once you're done, put r? @varkor in the pull request and I'll review it. Thanks!

@matthew-russo
Copy link
Contributor

@varkor Awesome, thanks!

@matthew-russo
Copy link
Contributor

matthew-russo commented Aug 21, 2018

@varkor GenericArgMismatchErrorCode struct is also used for E0087, E0088, E0089, and E0090 -- too many and too few lifetime and type arguments for functions. Is this supposed to convert those to use E0107 as well ?

@varkor
Copy link
Member Author

varkor commented Aug 21, 2018

@mcr431: oh, I forgot about those — yes, these should be merged with E0107 too. Then we'll just have a single error for "you didn't give the right number of generic arguments" (the actual error message itself will give the details about which kind of generic argument, and how many, so we don't need to separate the error codes too). Thanks!

@matthew-russo
Copy link
Contributor

matthew-russo commented Aug 22, 2018

@varkor I ran tests and the ones related to the newly changed error codes have failed (along with others that I assume are related to the changed errors. Am I supposed to modify the tests as well?

@varkor
Copy link
Member Author

varkor commented Aug 22, 2018

@mcr431: if you run the tests with --bless, it should update most of them automatically. In some places you might need to modify the //~ ERROR comments in the test itself though. I'd expect quite a lot of test output to change, so don't worry if you end up needing to modify some of the tests.

@matthew-russo
Copy link
Contributor

@varkor understood. How should I handle the errors such as ui/error-codes/E0244.rs (as well as all other removed error codes). Should these tests be removed (and then added to ui/error-codes/E0107.rs) or should I just edit them to use 0107 error code? Not super familiar with the testing structure so not sure if this question makes any sense.

@varkor
Copy link
Member Author

varkor commented Aug 22, 2018

@mcr431: ah, I forgot about those. I would edit them to use the E0107 error code (the other deprecated error codes still seem to have tests).

@varkor
Copy link
Member Author

varkor commented Aug 22, 2018

@mcr431: taking a closer look, it seems E0001 is the only deprecated error code that still has a test, so you can get away with just removing the deprecated error codes entirely. Sorry!

(You could probably also remove the test for E0001 while you're at it.)

@matthew-russo
Copy link
Contributor

Didn't realize you said I should remove E0001 and already submitted PR. let me know If i should modify to remove it

@varkor
Copy link
Member Author

varkor commented Aug 22, 2018

Didn't realize you said I should remove E0001 and already submitted PR.

Don't worry about it; it's technically unrelated.

bors added a commit that referenced this issue Aug 25, 2018
Fix #53525  - Unify E0243, E0244, E0087, E0088, E0089, and E0090 into E0107

Fix #53525

This pr merges all errors related to too many or too few generic arguments in types and functions. E0243, E0244, E0087, E0088, E0089, E0090 errors will no longer be emitted and E0107 will be used instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Development

No branches or pull requests

3 participants