From 476ea9ef1c0ba7250e369a2ec7205506b09474b7 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 13 May 2019 01:37:04 -0400 Subject: [PATCH] Always try to project predicates when finding auto traits in rustdoc 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 !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 { field: T } impl MyTrait for u8 { type Project = False; } unsafe impl Send for MyStruct where T: MyTrait {} pub struct Wrapper { inner: MyStruct } ``` 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. --- src/librustc/traits/auto_trait.rs | 85 ++++++++++++++++++++++++------- src/test/rustdoc/issue-60726.rs | 35 +++++++++++++ 2 files changed, 103 insertions(+), 17 deletions(-) create mode 100644 src/test/rustdoc/issue-60726.rs diff --git a/src/librustc/traits/auto_trait.rs b/src/librustc/traits/auto_trait.rs index 57f5e239188..2fa896962da 100644 --- a/src/librustc/traits/auto_trait.rs +++ b/src/librustc/traits/auto_trait.rs @@ -700,22 +700,64 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { } } - // 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 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { 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 index 00000000000..6acc8627738 --- /dev/null +++ b/src/test/rustdoc/issue-60726.rs @@ -0,0 +1,35 @@ +use std::marker::PhantomData; + +pub struct True; +pub struct False; + +pub trait InterfaceType{ + type Send; +} + + +pub struct FooInterface(PhantomDataT>); + +impl InterfaceType for FooInterface { + type Send=False; +} + + +pub struct DynTrait{ + _interface:PhantomDataI>, + _unsync_unsend:PhantomData<::std::rc::Rc<()>>, +} + +unsafe impl Send for DynTrait +where + I:InterfaceType +{} + +// @has issue_60726/struct.IntoIter.html +// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl !Send for \ +// IntoIter" +// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl !Sync for \ +// IntoIter" +pub struct IntoIter{ + hello:DynTrait>, +}