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 run coherence twice for future-compat lints #69044

Merged
merged 1 commit into from
Feb 11, 2020
Merged

Don't run coherence twice for future-compat lints #69044

merged 1 commit into from
Feb 11, 2020

Conversation

jonas-schievink
Copy link
Contributor

@jonas-schievink jonas-schievink commented Feb 10, 2020

This fixes the regression introduced by #65232 (which I mentioned in #65232 (comment)).

Old algorithm:

  • Run coherence with all future-incompatible checks off, reporting errors on any overlap.
  • If there's no overlap (common case), run it again, with the future-incompatible checks on. Report warnings for any overlap found.

New algorithm:

  • Run coherence with all additional future-incompatible checks on, which means that we'll find all potentially overlapping impls immediately.
  • If this found overlap, run coherence again, with the future-incompatible checks off. If that still gives an error, we report it. If not, it ought to be a warning.

This reduces time spent in coherence checking for the nrf52810-pac by roughly 50% relative to current master.

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(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 Feb 10, 2020
@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 11, 2020

📌 Commit f667937 has been approved by davidtwco

@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 Feb 11, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 11, 2020
…twice, r=davidtwco

Don't run coherence twice for future-compat lints

This fixes the regression introduced by rust-lang#65232 (which I mentioned in rust-lang#65232 (comment)).

Old algorithm:
* Run coherence with all future-incompatible checks off, reporting errors on any overlap.
* If there's no overlap (common case), run it *again*, with the future-incompatible checks on. Report warnings for any overlap found.

New algorithm:
* Run coherence with all additional future-incompatible checks *on*, which means that we'll find *all* potentially overlapping impls immediately.
* If this found overlap, run coherence again, with the future-incompatible checks off. If that *still* gives an error, we report it. If not, it ought to be a warning.

This reduces time spent in coherence checking for the nrf52810-pac by roughly 50% relative to current master.
bors added a commit that referenced this pull request Feb 11, 2020
Rollup of 8 pull requests

Successful merges:

 - #66498 (Remove unused feature gates)
 - #68816 (Tweak borrow error on `FnMut` when `Fn` is expected)
 - #68824 (Enable Control Flow Guard in rustbuild)
 - #69022 (traits: preallocate 2 Vecs of known initial size)
 - #69031 (Use `dyn Trait` more in tests)
 - #69044 (Don't run coherence twice for future-compat lints)
 - #69047 (Don't rustfmt check the vendor directory.)
 - #69055 (Clean up E0307 explanation)

Failed merges:

r? @ghost
@bors bors merged commit f667937 into rust-lang:master Feb 11, 2020
@jonas-schievink jonas-schievink deleted the dont-run-coherence-twice branch March 18, 2020 10:38
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.

4 participants