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

Don't emit the same compiler diagnostic twice. #45519

Merged
merged 4 commits into from
Oct 26, 2017

Conversation

michaelwoerister
Copy link
Member

This PR makes the compiler filter out diagnostic messages that have already been emitted during the same compilation session.

@rust-highfive
Copy link
Collaborator

r? @arielb1

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

@michaelwoerister
Copy link
Member Author

To clarify what motivates this PR: With incremental compilation there are cases where we emit diagnostics that we have cached from the previous compilation session. It then sometimes occurs that we have to recompute a value (because we have not cached it) but cannot decide for sure if we should re-emit diagnostics during this re-computation.

This PR solves the problem by having a global filter that just remembers all diagnostics that have already been emitted.

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 25, 2017
@arielb1
Copy link
Contributor

arielb1 commented Oct 25, 2017

So this is only needed when anonymous nodes (today trait selection) can emit warnings? Trait selection is already called very unpredictably, so emitting warnings from there is a good way to spam the user. I think that every place we want to use anonymous nodes in is going to be evaluated unpredictably (because we can't come with a good set of dep-nodes that should all be evaluated) and therefore would not be a good place to emit warnings from.

@michaelwoerister
Copy link
Member Author

It also solves #34229 and makes the implementation of this deduplication logic cleaner.

@arielb1
Copy link
Contributor

arielb1 commented Oct 25, 2017

That's a hacky but ok way of dealing with the derive problem
@bors r+

@bors
Copy link
Contributor

bors commented Oct 25, 2017

📌 Commit 9736474 has been approved by arielb1

@kennytm kennytm 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 Oct 26, 2017
@bors
Copy link
Contributor

bors commented Oct 26, 2017

⌛ Testing commit 9736474 with merge b218a02...

bors added a commit that referenced this pull request Oct 26, 2017
Don't emit the same compiler diagnostic twice.

This PR makes the compiler filter out diagnostic messages that have already been emitted during the same compilation session.
@bors
Copy link
Contributor

bors commented Oct 26, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing b218a02 to master...

@bors bors merged commit 9736474 into rust-lang:master Oct 26, 2017
bors added a commit that referenced this pull request Nov 1, 2017
…omatsakis

incr.comp.: Implement compiler diagnostic persistence.

This PR implements storing and loading diagnostics that the compiler generates and thus allows for emitting warnings during incremental compilation without actually re-evaluating the thing the warning originally came from. It also lays some groundwork for storing and loading type information and MIR in the incr. comp. cache.

~~It is still work in progress:~~
- ~~There's still some documentation to be added.~~
- ~~The way anonymous queries are handled might lead to duplicated emissions of warnings. Not sure if there is a better way or how frequent such duplication would be in practice.~~

Diagnostic message duplication is addressed separately in #45519.

r? @nikomatsakis
@durka
Copy link
Contributor

durka commented Nov 1, 2017

Hmm, after this PR the error count is wrong. For example, the code in #34229 stops after three unique errors, but says "aborting due to 9 previous errors".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

6 participants