]> git.lizzy.rs Git - rust.git/commitdiff
Always try to project predicates when finding auto traits in rustdoc
authorAaron Hill <aa1ronham@gmail.com>
Mon, 13 May 2019 05:37:04 +0000 (01:37 -0400)
committerAaron Hill <aa1ronham@gmail.com>
Mon, 13 May 2019 05:52:04 +0000 (01:52 -0400)
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<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.

src/librustc/traits/auto_trait.rs
src/test/rustdoc/issue-60726.rs [new file with mode: 0644]

index 57f5e23918884beee1911e713490425b8ca12969..2fa896962daf9f6c5442241ee41188b62bdd0409 100644 (file)
@@ -700,22 +700,64 @@ fn evaluate_nested_obligations<
                             }
                     }
 
-                    // We can only call poly_project_and_unify_type when our predicate's
-                    // Ty contains an inference variable - otherwise, there won't be anything to
-                    // unify
-                    if p.ty().skip_binder().has_infer_types() {
-                        debug!("Projecting and unifying projection predicate {:?}",
-                               predicate);
-                        match poly_project_and_unify_type(select, &obligation.with(p)) {
-                            Err(e) => {
-                                debug!(
-                                    "evaluate_nested_obligations: Unable to unify predicate \
-                                     '{:?}' '{:?}', bailing out",
-                                    ty, e
-                                );
-                                return false;
-                            }
-                            Ok(Some(v)) => {
+                    // There are three possible cases when we project a predicate:
+                    //
+                    // 1. We encounter an error. This means that it's impossible for
+                    // our current type to implement the auto trait - there's bound
+                    // that we could add to our ParamEnv that would 'fix' this kind
+                    // of error, as it's not caused by an unimplemented type.
+                    //
+                    // 2. We succesfully project the predicate (Ok(Some(_))), generating
+                    //  some subobligations. We then process these subobligations
+                    //  like any other generated sub-obligations.
+                    //
+                    // 3. We receieve an 'ambiguous' result (Ok(None))
+                    // If we were actually trying to compile a crate,
+                    // we would need to re-process this obligation later.
+                    // However, all we care about is finding out what bounds
+                    // are needed for our type to implement a particular auto trait.
+                    // We've already added this obligation to our computed ParamEnv
+                    // above (if it was necessary). Therefore, we don't need
+                    // to do any further processing of the obligation.
+                    //
+                    // Note that we *must* try to project *all* projection predicates
+                    // we encounter, even ones without inference variable.
+                    // This ensures that we detect any projection errors,
+                    // which indicate that our type can *never* implement the given
+                    // auto trait. In that case, we will generate an explicit negative
+                    // impl (e.g. 'impl !Send for MyType'). However, we don't
+                    // try to process any of the generated subobligations -
+                    // they contain no new information, since we already know
+                    // that our type implements the projected-through trait,
+                    // and can lead to weird region issues.
+                    //
+                    // Normally, we'll generate a negative impl as a result of encountering
+                    // a type with an explicit negative impl of an auto trait
+                    // (for example, raw pointers have !Send and !Sync impls)
+                    // However, through some **interesting** manipulations of the type
+                    // system, it's actually possible to write a type that never
+                    // implements an auto trait due to a projection error, not a normal
+                    // negative impl error. To properly handle this case, we need
+                    // to ensure that we catch any potential projection errors,
+                    // and turn them into an explicit negative impl for our type.
+                    debug!("Projecting and unifying projection predicate {:?}",
+                           predicate);
+
+                    match poly_project_and_unify_type(select, &obligation.with(p)) {
+                        Err(e) => {
+                            debug!(
+                                "evaluate_nested_obligations: Unable to unify predicate \
+                                 '{:?}' '{:?}', bailing out",
+                                ty, e
+                            );
+                            return false;
+                        }
+                        Ok(Some(v)) => {
+                            // We only care about sub-obligations
+                            // when we started out trying to unify
+                            // some inference variables. See the comment above
+                            // for more infomration
+                            if p.ty().skip_binder().has_infer_types() {
                                 if !self.evaluate_nested_obligations(
                                     ty,
                                     v.clone().iter().cloned(),
@@ -728,7 +770,16 @@ fn evaluate_nested_obligations<
                                     return false;
                                 }
                             }
-                            Ok(None) => {
+                        }
+                        Ok(None) => {
+                            // It's ok not to make progress when hvave no inference variables -
+                            // in that case, we were only performing unifcation to check if an
+                            // error occured (which would indicate that it's impossible for our
+                            // type to implement the auto trait).
+                            // However, we should always make progress (either by generating
+                            // subobligations or getting an error) when we started off with
+                            // inference variables
+                            if p.ty().skip_binder().has_infer_types() {
                                 panic!("Unexpected result when selecting {:?} {:?}", ty, obligation)
                             }
                         }
diff --git a/src/test/rustdoc/issue-60726.rs b/src/test/rustdoc/issue-60726.rs
new file mode 100644 (file)
index 0000000..6acc862
--- /dev/null
@@ -0,0 +1,35 @@
+use std::marker::PhantomData;
+
+pub struct True;
+pub struct False;
+
+pub trait InterfaceType{
+    type Send;
+}
+
+
+pub struct FooInterface<T>(PhantomData<fn()->T>);
+
+impl<T> InterfaceType for FooInterface<T> {
+    type Send=False;
+}
+
+
+pub struct DynTrait<I>{
+    _interface:PhantomData<fn()->I>,
+    _unsync_unsend:PhantomData<::std::rc::Rc<()>>,
+}
+
+unsafe impl<I> Send for DynTrait<I>
+where
+    I:InterfaceType<Send=True>
+{}
+
+// @has issue_60726/struct.IntoIter.html
+// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<T> !Send for \
+// IntoIter<T>"
+// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<T> !Sync for \
+// IntoIter<T>"
+pub struct IntoIter<T>{
+    hello:DynTrait<FooInterface<T>>,
+}