Coherence should allow fundamental types to impl traits when they are local#65738

ohadravid · 2019-10-23T21:26:06Z

After #64414, impl<T> Remote for Box<T> { } is disallowed, but it is also disallowed in liballoc, where Box is a local type!

Enabling #![feature(re_rebalance_coherence)] in liballoc results in:

error[E0210]: type parameter `F` must be used as the type parameter for some local type (e.g., `MyStruct<F>`)
    --> src\liballoc\boxed.rs:1098:1
     |
1098 | impl<F: ?Sized + Future + Unpin> Future for Box<F> {
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `F` must be used as the type parameter for some local type

This PR relaxes uncover_fundamental_ty to skip local fundamental types.
I didn't add a test since liballoc already fails to compile, but I can add one if needed.

r? @nikomatsakis

cc #63599

nikomatsakis · 2019-10-25T20:34:39Z

Heh, good catch @ohadravid -- I'm torn about the test. You're right that we don't need one in some sense but I think if you don't mind adding one it would help us to prevent similar oversights in the future.

ohadravid · 2019-10-26T11:21:12Z

@nikomatsakis I added two tests which should prevent a regression.

BTW I can also open the stabilization PR for re_rebalance_coherence, I just wasn't sure if it was taken or not.

Original failures are:

LL | impl Remote for Local {}
   | ^^^^^^^^^^^^^^^^^^^^^ impl doesn't use types inside crate

and

LL | impl<T> Remote for MyBox<T> {}                                                                             
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type

nikomatsakis · 2019-10-26T14:42:14Z

@bors r+

bors · 2019-10-26T14:42:16Z

📌 Commit 8f988bd has been approved by nikomatsakis

nikomatsakis · 2019-10-26T14:42:43Z

@ohadravid thanks! And yes, a stabilization PR would be great! Please explicitly add r? @nikomatsakis on the PR, too.

@nikomatsakis

…low-fundamental-local, r=nikomatsakis Coherence should allow fundamental types to impl traits when they are local After rust-lang#64414, `impl<T> Remote for Box<T> { }` is disallowed, but it is also disallowed in liballoc, where `Box` is a local type! Enabling `#![feature(re_rebalance_coherence)]` in `liballoc` results in: ``` error[E0210]: type parameter `F` must be used as the type parameter for some local type (e.g., `MyStruct<F>`) --> src\liballoc\boxed.rs:1098:1 | 1098 | impl<F: ?Sized + Future + Unpin> Future for Box<F> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `F` must be used as the type parameter for some local type ``` This PR relaxes `uncover_fundamental_ty` to skip local fundamental types. I didn't add a test since `liballoc` already fails to compile, but I can add one if needed. r? @nikomatsakis cc rust-lang#63599

@nikomatsakis

…low-fundamental-local, r=nikomatsakis Coherence should allow fundamental types to impl traits when they are local After rust-lang#64414, `impl<T> Remote for Box<T> { }` is disallowed, but it is also disallowed in liballoc, where `Box` is a local type! Enabling `#![feature(re_rebalance_coherence)]` in `liballoc` results in: ``` error[E0210]: type parameter `F` must be used as the type parameter for some local type (e.g., `MyStruct<F>`) --> src\liballoc\boxed.rs:1098:1 | 1098 | impl<F: ?Sized + Future + Unpin> Future for Box<F> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `F` must be used as the type parameter for some local type ``` This PR relaxes `uncover_fundamental_ty` to skip local fundamental types. I didn't add a test since `liballoc` already fails to compile, but I can add one if needed. r? @nikomatsakis cc rust-lang#63599

@nikomatsakis

…low-fundamental-local, r=nikomatsakis Coherence should allow fundamental types to impl traits when they are local After rust-lang#64414, `impl<T> Remote for Box<T> { }` is disallowed, but it is also disallowed in liballoc, where `Box` is a local type! Enabling `#![feature(re_rebalance_coherence)]` in `liballoc` results in: ``` error[E0210]: type parameter `F` must be used as the type parameter for some local type (e.g., `MyStruct<F>`) --> src\liballoc\boxed.rs:1098:1 | 1098 | impl<F: ?Sized + Future + Unpin> Future for Box<F> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `F` must be used as the type parameter for some local type ``` This PR relaxes `uncover_fundamental_ty` to skip local fundamental types. I didn't add a test since `liballoc` already fails to compile, but I can add one if needed. r? @nikomatsakis cc rust-lang#63599

@ghost

Rollup of 6 pull requests Successful merges: - #65566 (Use heuristics to suggest assignment) - #65738 (Coherence should allow fundamental types to impl traits when they are local) - #65777 (Don't ICE for completely unexpandable `impl Trait` types) - #65834 (Remove lint callback from driver) - #65839 (Clean up `check_consts` now that new promotion pass is implemented) - #65855 (Add long error explaination for E0666) Failed merges: r? @ghost

bors · 2019-10-27T19:42:59Z

☔ The latest upstream changes (presumably #65869) made this pull request unmergeable. Please resolve the merge conflicts.

rust-highfive assigned nikomatsakis Oct 23, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 23, 2019

ohadravid force-pushed the re-rebalance-coherence-allow-fundamental-local branch from f36742a to 67953fd Compare October 23, 2019 21:40

Centril added the F-re_rebalance_coherence `#![feature(re_rebalance_coherence)]` label Oct 24, 2019

Coherence should allow fundamental types to impl traits

8f988bd

ohadravid force-pushed the re-rebalance-coherence-allow-fundamental-local branch from 67953fd to 8f988bd Compare October 26, 2019 11:09

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 Oct 26, 2019

Centril mentioned this pull request Oct 26, 2019

Rollup of 6 pull requests #65851

Closed

Centril mentioned this pull request Oct 26, 2019

Rollup of 7 pull requests #65854

Closed

Centril mentioned this pull request Oct 27, 2019

Rollup of 6 pull requests #65869

Merged

bors merged commit 8f988bd into rust-lang:master Oct 27, 2019

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 27, 2019

ohadravid deleted the re-rebalance-coherence-allow-fundamental-local branch December 2, 2019 11:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Coherence should allow fundamental types to impl traits when they are local#65738