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

Incorrect generic parameter ordering for E0747 #72815

Closed
varkor opened this issue May 31, 2020 · 6 comments · Fixed by #72848
Closed

Incorrect generic parameter ordering for E0747 #72815

varkor opened this issue May 31, 2020 · 6 comments · Fixed by #72848
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. 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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@varkor
Copy link
Member

varkor commented May 31, 2020

https://github.com/rust-lang/rust/blob/a59264b01247836c70e24217e0d346b868387525/src/test/ui/suggestions/suggest-move-types.stderr#L121-L135
suggests type parameters should come before lifetime parameters, which is incorrect.
The message is emitted here:
https://github.com/rust-lang/rust/blob/a59264b01247836c70e24217e0d346b868387525/src/librustc_typeck/astconv.rs#L463-L477

This issue has been assigned to @camelid via this comment.

@varkor varkor added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-diagnostics Area: Messages for errors, warnings, and lints E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. labels May 31, 2020
@camelid
Copy link
Member

camelid commented May 31, 2020

I want to try working on this.

@rustbot claim

@rustbot rustbot self-assigned this May 31, 2020
@camelid
Copy link
Member

camelid commented May 31, 2020

I've been trying to fix this, but even reversing the arguments to the err.note format didn't seem to do anything. Am I missing something?

I also added ABC to the error note for testing:

err.note(&format!("{} arguments must be provided before {} arguments ABC", arg.descr(), kind));

And here's the error diff:

---- [ui] ui/suggestions/suggest-move-types.rs stdout ----
diff of stderr:

108	LL | struct Al<'a, T, M: OneWithLifetime<A=(), T, 'a>> {
109	   |                                           ^
110	   |
-	   = note: lifetime arguments must be provided before type arguments
+	   = note: type arguments must be provided before lifetime arguments ABC
112	
113	error[E0747]: type provided when a lifetime was expected
114	  --> $DIR/suggest-move-types.rs:48:71

116	LL | struct Bl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<A=(), B=(), C=(), T, U, V, 'a, 'b, 'c>> {
117	   |                                                                       ^
118	   |
-	   = note: lifetime arguments must be provided before type arguments
+	   = note: type arguments must be provided before lifetime arguments ABC
120	
121	error[E0747]: lifetime provided when a type was expected
122	  --> $DIR/suggest-move-types.rs:65:56

124	LL | struct Cl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<T, 'a, A=(), B=(), C=(), U, 'b, V, 'c>> {
125	   |                                                        ^^
126	   |
-	   = note: lifetime arguments must be provided before type arguments
+	   = note: lifetime arguments must be provided before type arguments ABC
128	
129	error[E0747]: lifetime provided when a type was expected
130	  --> $DIR/suggest-move-types.rs:82:56

132	LL | struct Dl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<T, 'a, A=(), B=(), U, 'b, C=(), V, 'c>> {
133	   |                                                        ^^
134	   |
-	   = note: lifetime arguments must be provided before type arguments
+	   = note: lifetime arguments must be provided before type arguments ABC
136	
137	error: aborting due to 12 previous errors
138	


The actual stderr differed from the expected stderr.

EDIT: I modified the expected stderr for the test so that it matches how it should look after this is fixed by the way.

@camelid
Copy link
Member

camelid commented May 31, 2020

I've looked into it a bit more, and it looks like this isn't as simple as I thought at first. I thought it might work if I just reversed the arguments to format!, but that messes up other things.

I think I'll need to rethink this.

@varkor
Copy link
Member Author

varkor commented May 31, 2020

I've been trying to fix this, but even reversing the arguments to the err.note format didn't seem to do anything. Am I missing something?

Note that I cherry-picked the examples that were incorrect. The diagnostic is correct in other cases. You'll need to investigate a little to work out how best to fix this; one simple solution would be to sort the two with ParamKindOrd, though I'm not sure if that's the cleanest approach.

@camelid
Copy link
Member

camelid commented Jun 1, 2020

Okay, thanks for the info! I'll look into sorting and see if that works :)

@camelid
Copy link
Member

camelid commented Jun 1, 2020

I think I've figured out a basic solution. Let me know if there are things I can improve!

@bors bors closed this as completed in 69a1ac3 Jun 3, 2020
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 D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. 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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants