Skip to content

Always try to project predicates when finding auto traits in rustdoc - #60773

Merged
bors merged 1 commit into
rust-lang:masterfrom
Aaron1011:fix/rustdoc-project-all
May 23, 2019
Merged

Always try to project predicates when finding auto traits in rustdoc#60773
bors merged 1 commit into
rust-lang:masterfrom
Aaron1011:fix/rustdoc-project-all

Conversation

@Aaron1011

Copy link
Copy Markdown
Contributor

Fixes#60726

Previous, AutoTraitFinder would only try to project predicates when the
predicate type contained an inference variable. When finding auto
traits, we only project to try to unify inference variables - we don't
otherwise learn any new information about the required bounds.

However, this lead to failing to properly generate a negative auto trait
impl (indicating that a type never implements a certain auto trait) in
the following unusual scenario:

In almost all cases, a type has an (implicit) negative impl of an auto
trait due some other type having an explicit negative impl of that
auto trait. For example:

struct MyType {
field: *const T
}

has an implicit 'impl !Send for MyType', due to the explicit
negative impl (in libcore) 'impl<T: ?Sized> !Send for *const T'.

However, as exposed by the 'abi_stable' crate, this isn't always the
case. This minimzed example shows how a type can never implement
'Send', due to a projection error:

pub struct True;
pub struct False;
pub trait MyTrait {
type Project;
}
pub struct MyStruct<T> {
field: T
}
impl MyTrait for u8 {
type Project = False;
}
unsafe impl<T> Send for MyStruct<T>
where T: MyTrait<Project=True> {}
pub struct Wrapper {
inner: MyStruct<u8>
}

In this example, `::Project == True'
must hold for 'MyStruct: Send' to hold.
However, '::Project == False' holds instead

To properly account for this unusual case, we need to call
'poly_project_and_unify' on all predicates, not just those with
inference variables. This ensures that we catch the projection error
that occurs above, and don't incorrectly determine that 'Wrapper: Send'
holds.

Fixesrust-lang#60726
Previous, AutoTraitFinder would only try to project predicates when the
predicate type contained an inference variable. When finding auto
traits, we only project to try to unify inference variables - we don't
otherwise learn any new information about the required bounds.
However, this lead to failing to properly generate a negative auto trait
impl (indicating that a type never implements a certain auto trait) in
the following unusual scenario:
In almost all cases, a type has an (implicit) negative impl of an auto
trait due some other type having an explicit *negative* impl of that
auto trait. For example:
struct MyType<T> {
field: *const T
}
has an implicit 'impl<T> !Send for MyType<T>', due to the explicit
negative impl (in libcore) 'impl<T: ?Sized> !Send for *const T'.
However, as exposed by the 'abi_stable' crate, this isn't always the
case. This minimzed example shows how a type can never implement
'Send', due to a projection error:
```
pub struct True;
pub struct False;
pub trait MyTrait {
type Project;
}
pub struct MyStruct<T> {
field: T
}
impl MyTrait for u8 {
type Project = False;
}
unsafe impl<T> Send for MyStruct<T>
where T: MyTrait<Project=True> {}
pub struct Wrapper {
inner: MyStruct<u8>
}
```
In this example, `<u8 as MyTrait>::Project == True'
must hold for 'MyStruct<u8>: Send' to hold.
However, '<u8 as MyTrait>::Project == False' holds instead
To properly account for this unusual case, we need to call
'poly_project_and_unify' on *all* predicates, not just those with
inference variables. This ensures that we catch the projection error
that occurs above, and don't incorrectly determine that 'Wrapper: Send'
holds.
@rust-highfive

Copy link
Copy Markdown
Contributor

r? @eddyb

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

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2019
@hellow554

Copy link
Copy Markdown
Contributor

Would you mind reducing the testcase a little bit?

pubstructTrue;pubstructFalse;pubtraitInterfaceType{typeSend;}pubstructFooInterface;pubstructDynTrait<I>(I);pubstructIntoIter(DynTrait<FooInterface>);implInterfaceTypeforFooInterface{typeSend = False;}unsafeimpl<I>SendforDynTrait<I>whereI:InterfaceType<Send = True>{}

@Dylan-DPC-zz

Copy link
Copy Markdown

ping from triage @eddyb waiting for your review on this

@eddybeddyb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm a bit dubious on this whole module but AFAIK it only affects rustdoc right now.

@eddyb

Copy link
Copy Markdown
Contributor

cc @rust-lang/wg-traits You might want to review this change at some point (and the whole module - maybe it can be simplified/deduplicated with other parts of the trait system?)

@bors r+

@bors

bors commented May 22, 2019

Copy link
Copy Markdown
Collaborator

📌 Commit 476ea9e has been approved by eddyb

@borsbors 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 May 22, 2019
Centril added a commit to Centril/rust that referenced this pull request May 22, 2019
…r=eddyb
Always try to project predicates when finding auto traits in rustdoc
Fixesrust-lang#60726
Previous, AutoTraitFinder would only try to project predicates when the
predicate type contained an inference variable. When finding auto
traits, we only project to try to unify inference variables - we don't
otherwise learn any new information about the required bounds.
However, this lead to failing to properly generate a negative auto trait
impl (indicating that a type never implements a certain auto trait) in
the following unusual scenario:
In almost all cases, a type has an (implicit) negative impl of an auto
trait due some other type having an explicit *negative* impl of that
auto trait. For example:
struct MyType<T> {
field: *const T
}
has an implicit 'impl<T> !Send for MyType<T>', due to the explicit
negative impl (in libcore) 'impl<T: ?Sized> !Send for *const T'.
However, as exposed by the 'abi_stable' crate, this isn't always the
case. This minimzed example shows how a type can never implement
'Send', due to a projection error:
```
pub struct True;
pub struct False;
pub trait MyTrait {
type Project;
}
pub struct MyStruct<T> {
field: T
}
impl MyTrait for u8 {
type Project = False;
}
unsafe impl<T> Send for MyStruct<T>
where T: MyTrait<Project=True> {}
pub struct Wrapper {
inner: MyStruct<u8>
}
```
In this example, `<u8 as MyTrait>::Project == True'
must hold for 'MyStruct<u8>: Send' to hold.
However, '<u8 as MyTrait>::Project == False' holds instead
To properly account for this unusual case, we need to call
'poly_project_and_unify' on *all* predicates, not just those with
inference variables. This ensures that we catch the projection error
that occurs above, and don't incorrectly determine that 'Wrapper: Send'
holds.
@CentrilCentril mentioned this pull request May 22, 2019
bors added a commit that referenced this pull request May 22, 2019
Rollup of 8 pull requests
Successful merges:
- #60300 (Allow null-pointer-optimized enums in FFI if their underlying representation is FFI safe)
- #60773 (Always try to project predicates when finding auto traits in rustdoc)
- #60809 (Add FAQ for NLL migration)
- #61023 (Migrate from recursion to iterate on qualify consts visitor impl)
- #61029 (Simplify RefCell minimum_spanning_tree example)
- #61030 (Make maybe_codegen_consume_direct iterate instead of doing recursion)
- #61034 (rustc_metadata: parametrize schema::CrateRoot by 'tcx and rip out old unused incremental infra.)
- #61037 (Update clippy submodule)
Failed merges:
r? @ghost
@bors
bors merged commit 476ea9e into rust-lang:masterMay 23, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: 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.

rustdoc:ICE with field that has a conditional Send impl.

6 participants

@Aaron1011@rust-highfive@hellow554@Dylan-DPC-zz@eddyb@bors