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.

Sign up for GitHub

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

Jump to bottom

Add long explanation for E0212 #79639

Merged
merged 3 commits into from
Dec 11, 2020
Merged

Add long explanation for E0212 #79639

merged 3 commits into from
Dec 11, 2020

Conversation

sasurau4
Copy link
Contributor

@sasurau4 sasurau4 commented Dec 2, 2020

Helps with #61137

@rust-highfive
Copy link
Contributor

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

@rust-highfive
Copy link
Contributor

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 2, 2020
@jyn514 jyn514 added the A-diagnostics Area: Messages for errors, warnings, and lints label Dec 2, 2020
@jyn514 jyn514 added A-lifetimes Area: Lifetimes / regions T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 2, 2020
@sasurau4 sasurau4 requested a review from jyn514 December 3, 2020 09:56
jyn514
jyn514 reviewed Dec 3, 2020
View reviewed changes
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Squashing is good, but you can wait to squash until all the comments have been addressed if you like.

compiler/rustc_error_codes/src/error_codes/E0212.md Outdated
field: I::A) {} // error!


struct SomeStruct<I: for<'x> Foo<&'x isize>> {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe only have one example? These all look basically the same and I think it's more important to be concise than exhaustive here. I don't feel strongly about that though.

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 took these example from corresponding test cases. I agree with you that the important point is conciseness rather than exhaustiveness. Also, I want to know what other reviewers think about this.

@sasurau4 sasurau4 force-pushed the feature/add-long-explanation-E0212 branch from 532c10f to be34f7f Compare December 4, 2020 12:56
sasurau4 and others added 3 commits December 4, 2020 22:17
@sasurau4
Refine E0212 error message
36363e5
@sasurau4
Add long explanation for E0212
a1e94cd 
Update compiler/rustc_error_codes/src/error_codes/E0212.md

Co-authored-by: Joshua Nelson <[email protected]>
@sasurau4
Update some associated-types ui test suites
87c6216
@sasurau4 sasurau4 force-pushed the feature/add-long-explanation-E0212 branch from be34f7f to 87c6216 Compare December 4, 2020 13:17
@sasurau4 sasurau4 requested a review from jyn514 December 4, 2020 13:21
jyn514
jyn514 approved these changes Dec 4, 2020
View reviewed changes
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd like @estebank to double-check the new error message I suggested makes sense.

@Dylan-DPC-zz
Copy link

r? @GuillaumeGomez

@GuillaumeGomez
Copy link
Member

Looks good to me. Do you want to squash or not here? In any case, r=me once ready.

@jyn514
Copy link
Member

jyn514 commented Dec 9, 2020

@bors delegate=@sasurau4

@bors
Copy link
Collaborator

bors commented Dec 9, 2020

✌️ @@sasurau4 can now approve this pull request

@sasurau4
Copy link
Contributor Author

@bors r=GuillaumeGomez rollup

@bors
Copy link
Collaborator

bors commented Dec 10, 2020

@sasurau4: 🔑 Insufficient privileges: Not in reviewers

@bors
Copy link
Collaborator

bors commented Dec 10, 2020

@sasurau4: 🔑 Insufficient privileges: not in try users

@sasurau4
Copy link
Contributor Author

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 10, 2020

@sasurau4: 🔑 Insufficient privileges: Not in reviewers

@bors
Copy link
Collaborator

bors commented Dec 10, 2020

@sasurau4: 🔑 Insufficient privileges: not in try users

@jyn514
Copy link
Member

jyn514 commented Dec 10, 2020

😕

@bors r=GuillaumeGomez rollup

@bors
Copy link
Collaborator

bors commented Dec 10, 2020

📌 Commit 87c6216 has been approved by GuillaumeGomez

@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 Dec 10, 2020
@JohnTitor JohnTitor mentioned this pull request Dec 10, 2020
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 10, 2020
@Dylan-DPC
Rollup merge of rust-lang#79639 - sasurau4:feature/add-long-explanati…
c124229 
…on-E0212, r=GuillaumeGomez

Add long explanation for E0212

Helps with rust-lang#61137
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Dec 10, 2020
Closed
@tmandry tmandry mentioned this pull request Dec 11, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 11, 2020
@bors
Auto merge of rust-lang#79927 - tmandry:rollup-pwn4b1v, r=tmandry
65d053a 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#77027 (Improve documentation for `std::{f32,f64}::mul_add`)
 - rust-lang#79375 (Make the kernel_copy tests more robust/concurrent.)
 - rust-lang#79639 (Add long explanation for E0212)
 - rust-lang#79698 (Add tracking issue template for library features.)
 - rust-lang#79809 (Dogfood `str_split_once()`)
 - rust-lang#79851 (Clarify the 'default is only allowed on...' error)
 - rust-lang#79858 (Update const-fn doc in unstable-book)
 - rust-lang#79860 (Clarify that String::split_at takes a byte index.)
 - rust-lang#79871 (Fix small typo in `wrapping_shl` documentation)
 - rust-lang#79896 (Make search results tab and help button focusable with keyboard)
 - rust-lang#79917 (Use Symbol for inline asm register class names)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f3a3fc9 into rust-lang:master Dec 11, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 11, 2020
@sasurau4 sasurau4 deleted the feature/add-long-explanation-E0212 branch December 11, 2020 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

@jyn514 jyn514 jyn514 approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: Lifetimes / regions S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.50.0
Development

Successfully merging this pull request may close these issues.
8 participants
@sasurau4 @rust-highfive @Dylan-DPC-zz @GuillaumeGomez @jyn514 @bors @estebank @rustbot