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

Report full details of inference errors #39913

Merged
merged 3 commits into from
Feb 20, 2017

Conversation

nikomatsakis
Copy link
Contributor

When the old suggestion machinery was removed by @brson in #37057, it was not completely removed. There was a bit of code that had the job of going through errors and finding those for which suggestions were applicable, and it remained, causing us not to emit the full details of such errors. This PR removes that.

I've also added various lifetime tests to the UI test suite (so you can also see the before/after there). I have some concrete thoughts on how to improve these cases and am planning on writing those up in some mentoring issues (@cengizio has expressed interest in working on those changes, so I plan to work with him on it, at least to start).

cc @jonathandturner

These are some samples that I have been focusing on improving over
time. In this PR, I mainly want to stem the bleeding where we in some
cases we show an error that gives you no possible way to divine the
problem.
@rust-highfive
Copy link
Collaborator

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@nikomatsakis
Copy link
Contributor Author

Nominated for beta consideration. I think the "curtailed" errors are in beta now.

@nikomatsakis nikomatsakis added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 20, 2017
@pnkfelix
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 20, 2017

📌 Commit 75da4b6 has been approved by pnkfelix

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 20, 2017
…felix

Report full details of inference errors

When the old suggestion machinery was removed by @brson in rust-lang#37057, it was not completely removed. There was a bit of code that had the job of going through errors and finding those for which suggestions were applicable, and it remained, causing us not to emit the full details of such errors.  This PR removes that.

I've also added various lifetime tests to the UI test suite (so you can also see the before/after there). I have some concrete thoughts on how to improve these cases and am planning on writing those up in some mentoring issues (@cengizio has expressed interest in working on those changes, so I plan to work with him on it, at least to start).

cc @jonathandturner
bors added a commit that referenced this pull request Feb 20, 2017
Rollup of 3 pull requests

- Successful merges: #39913, #39937, #39976
- Failed merges:
@bors bors merged commit 75da4b6 into rust-lang:master Feb 20, 2017
@brson brson added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Feb 23, 2017
@brson
Copy link
Contributor

brson commented Feb 23, 2017

Seems like following through in this is appropriate for beta.

@brson brson mentioned this pull request Feb 23, 2017
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 23, 2017
bors added a commit that referenced this pull request Mar 2, 2017
[beta] next

- #39913
- #39730
- #39674
- #39602
- #39586
- #39471
- #39980
- #40020
- #40135

@nikomatsakis [this commit](3787d33) did not pick cleanly. You might peek at it.

I took the liberty of accepting all the nominations myself, but the [packed struct alignment](#39586) PR is quite large. It did pick fine though and there's a comment there suggesting it works on beta cc @rust-lang/compiler.

cc @alexcrichton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants