From f57247c48cb59a59dcfcb220251206064265479c Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 24 Oct 2018 00:57:09 -0400 Subject: [PATCH 1/4] Ensure that Rusdoc discovers all necessary auto trait bounds Fixes #50159 This commit makes several improvements to AutoTraitFinder: * Call infcx.resolve_type_vars_if_possible before processing new predicates. This ensures that we eliminate inference variables wherever possible. * Process all nested obligations we get from a vtable, not just ones with depth=1. * The 'depth=1' check was a hack to work around issues processing certain predicates. The other changes in this commit allow us to properly process all predicates that we encounter, so the check is no longer necessary, * Ensure that we only display predicates *without* inference variables to the user, and only attempt to unify predicates that *have* an inference variable as their type. Additionally, the internal helper method is_of_param now operates directly on a type, rather than taking a Substs. This allows us to use the 'self_ty' method, rather than directly dealing with Substs. --- src/librustc/traits/auto_trait.rs | 68 +++++++++++++++++++++++-------- src/test/rustdoc/issue-50159.rs | 31 ++++++++++++++ 2 files changed, 82 insertions(+), 17 deletions(-) create mode 100644 src/test/rustdoc/issue-50159.rs diff --git a/src/librustc/traits/auto_trait.rs b/src/librustc/traits/auto_trait.rs index c3cca149c2c57..6279788adc019 100644 --- a/src/librustc/traits/auto_trait.rs +++ b/src/librustc/traits/auto_trait.rs @@ -334,7 +334,12 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { continue; } - let result = select.select(&Obligation::new(dummy_cause.clone(), new_env, pred)); + // Call infcx.resolve_type_vars_if_possible to see if we can + // get rid of any inference variables. + let obligation = infcx.resolve_type_vars_if_possible( + &Obligation::new(dummy_cause.clone(), new_env, pred) + ); + let result = select.select(&obligation); match &result { &Ok(Some(ref vtable)) => { @@ -369,7 +374,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { } &Ok(None) => {} &Err(SelectionError::Unimplemented) => { - if self.is_of_param(pred.skip_binder().trait_ref.substs) { + if self.is_of_param(pred.skip_binder().self_ty()) { already_visited.remove(&pred); self.add_user_pred( &mut user_computed_preds, @@ -631,14 +636,10 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { finished_map } - pub fn is_of_param(&self, substs: &Substs<'_>) -> bool { - if substs.is_noop() { - return false; - } - - return match substs.type_at(0).sty { + pub fn is_of_param(&self, ty: Ty<'_>) -> bool { + return match ty.sty { ty::Param(_) => true, - ty::Projection(p) => self.is_of_param(p.substs), + ty::Projection(p) => self.is_of_param(p.self_ty()), _ => false, }; } @@ -661,28 +662,61 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { ) -> bool { let dummy_cause = ObligationCause::misc(DUMMY_SP, ast::DUMMY_NODE_ID); - for (obligation, predicate) in nested - .filter(|o| o.recursion_depth == 1) + for (obligation, mut predicate) in nested .map(|o| (o.clone(), o.predicate.clone())) { let is_new_pred = fresh_preds.insert(self.clean_pred(select.infcx(), predicate.clone())); + // Resolve any inference variables that we can, to help selection succeed + predicate = select.infcx().resolve_type_vars_if_possible(&predicate); + + // We only add a predicate as a user-displayable bound if + // it involves a generic parameter, and doesn't contain + // any inference variables. + // + // Displaying a bound involving a concrete type (instead of a generic + // parameter) would be pointless, since it's always true + // (e.g. u8: Copy) + // Displaying an inference variable is impossible, since they're + // an internal compiler detail without a defined visual representation + // + // We check this by calling is_of_param on the relevant types + // from the various possible predicates match &predicate { &ty::Predicate::Trait(ref p) => { - let substs = &p.skip_binder().trait_ref.substs; + if self.is_of_param(p.skip_binder().self_ty()) + && !only_projections + && is_new_pred { - if self.is_of_param(substs) && !only_projections && is_new_pred { self.add_user_pred(computed_preds, predicate); } predicates.push_back(p.clone()); } &ty::Predicate::Projection(p) => { - // If the projection isn't all type vars, then - // we don't want to add it as a bound - if self.is_of_param(p.skip_binder().projection_ty.substs) && is_new_pred { + debug!("evaluate_nested_obligations: examining projection predicate {:?}", + predicate); + + // As described above, we only want to display + // bounds which include a generic parameter but don't include + // an inference variable. + // Additionally, we check if we've seen this predicate before, + // to avoid rendering duplicate bounds to the user. + if self.is_of_param(p.skip_binder().projection_ty.self_ty()) + && !p.ty().skip_binder().is_ty_infer() + && is_new_pred { + debug!("evaluate_nested_obligations: adding projection predicate\ + to computed_preds: {:?}", predicate); + self.add_user_pred(computed_preds, predicate); - } else { + } + + // We can only call poly_project_and_unify_type when our predicate's + // Ty is an inference variable - otherwise, there won't be anything to + // unify + if p.ty().skip_binder().is_ty_infer() { + debug!("Projecting and unifying projection predicate {:?}", + predicate); match poly_project_and_unify_type(select, &obligation.with(p.clone())) { Err(e) => { debug!( diff --git a/src/test/rustdoc/issue-50159.rs b/src/test/rustdoc/issue-50159.rs new file mode 100644 index 0000000000000..3055c72162452 --- /dev/null +++ b/src/test/rustdoc/issue-50159.rs @@ -0,0 +1,31 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + + +pub trait Signal { + type Item; +} + +pub trait Signal2 { + type Item2; +} + +impl Signal2 for B where B: Signal { + type Item2 = C; +} + +// @has issue_50159/struct.Switch.html +// @has - '//code' 'impl Send for Switch where ::Item: Send' +// @has - '//code' 'impl Sync for Switch where ::Item: Sync' +// @count - '//*[@id="implementations-list"]/*[@class="impl"]' 0 +// @count - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]' 2 +pub struct Switch { + pub inner: ::Item2, +} From 1a84d211a2dad78f1d3412c65f5a72053a82893d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 14 Nov 2018 16:36:48 -0500 Subject: [PATCH 2/4] Check all substitution parameters for inference variables --- src/librustc/traits/auto_trait.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/librustc/traits/auto_trait.rs b/src/librustc/traits/auto_trait.rs index 6279788adc019..f560772e6c73f 100644 --- a/src/librustc/traits/auto_trait.rs +++ b/src/librustc/traits/auto_trait.rs @@ -374,7 +374,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { } &Ok(None) => {} &Err(SelectionError::Unimplemented) => { - if self.is_of_param(pred.skip_binder().self_ty()) { + if self.is_param_no_infer(pred.skip_binder().trait_ref.substs) { already_visited.remove(&pred); self.add_user_pred( &mut user_computed_preds, @@ -636,6 +636,11 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { finished_map } + fn is_param_no_infer(&self, substs: &Substs<'_>) -> bool { + return self.is_of_param(substs.type_at(0)) && + !substs.types().any(|t| t.has_infer_types()); + } + pub fn is_of_param(&self, ty: Ty<'_>) -> bool { return match ty.sty { ty::Param(_) => true, @@ -685,7 +690,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { // from the various possible predicates match &predicate { &ty::Predicate::Trait(ref p) => { - if self.is_of_param(p.skip_binder().self_ty()) + if self.is_param_no_infer(p.skip_binder().trait_ref.substs) && !only_projections && is_new_pred { @@ -702,7 +707,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { // an inference variable. // Additionally, we check if we've seen this predicate before, // to avoid rendering duplicate bounds to the user. - if self.is_of_param(p.skip_binder().projection_ty.self_ty()) + if self.is_param_no_infer(p.skip_binder().projection_ty.substs) && !p.ty().skip_binder().is_ty_infer() && is_new_pred { debug!("evaluate_nested_obligations: adding projection predicate\ From 5045e12bd7001956b14a6dfd642637a471e4b378 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 28 Nov 2018 21:15:06 -0500 Subject: [PATCH 3/4] Filter out self-referential projection predicates If we end up with a projection predicate that equates a type with itself (e.g. ::Value == ::Value), we can run into issues if we try to add it to our ParamEnv. --- src/librustc/traits/auto_trait.rs | 27 ++++++++++++- .../synthetic_auto/self-referential.rs | 40 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 src/test/rustdoc/synthetic_auto/self-referential.rs diff --git a/src/librustc/traits/auto_trait.rs b/src/librustc/traits/auto_trait.rs index f560772e6c73f..a0237348ea690 100644 --- a/src/librustc/traits/auto_trait.rs +++ b/src/librustc/traits/auto_trait.rs @@ -649,6 +649,15 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { }; } + fn is_self_referential_projection(&self, p: ty::PolyProjectionPredicate<'_>) -> bool { + match p.ty().skip_binder().sty { + ty::Projection(proj) if proj == p.skip_binder().projection_ty => { + true + }, + _ => false + } + } + pub fn evaluate_nested_obligations< 'b, 'c, @@ -713,7 +722,23 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { debug!("evaluate_nested_obligations: adding projection predicate\ to computed_preds: {:?}", predicate); - self.add_user_pred(computed_preds, predicate); + // Under unusual circumstances, we can end up with a self-refeential + // projection predicate. For example: + // ::Value == ::Value + // Not only is displaying this to the user pointless, + // having it in the ParamEnv will cause an issue if we try to call + // poly_project_and_unify_type on the predicate, since this kind of + // predicate will normally never end up in a ParamEnv. + // + // For these reasons, we ignore these weird predicates, + // ensuring that we're able to properly synthesize an auto trait impl + if self.is_self_referential_projection(p) { + debug!("evaluate_nested_obligations: encountered a projection + predicate equating a type with itself! Skipping"); + + } else { + self.add_user_pred(computed_preds, predicate); + } } // We can only call poly_project_and_unify_type when our predicate's diff --git a/src/test/rustdoc/synthetic_auto/self-referential.rs b/src/test/rustdoc/synthetic_auto/self-referential.rs new file mode 100644 index 0000000000000..077786b280fc3 --- /dev/null +++ b/src/test/rustdoc/synthetic_auto/self-referential.rs @@ -0,0 +1,40 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Some unusual code minimized from +// https://github.com/sile/handy_async/tree/7b619b762c06544fc67792c8ff8ebc24a88fdb98 + +pub trait Pattern { + type Value; +} + +pub struct Constrain(A, B, C); + +impl Pattern for Constrain + where A: Pattern, + B: Pattern, + C: Pattern, +{ + type Value = A::Value; +} + +pub struct Wrapper(T); + +impl Pattern for Wrapper { + type Value = T; +} + + +// @has self_referential/struct.WriteAndThen.html +// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl Send for \ +// WriteAndThen where ::Value: Send" +pub struct WriteAndThen(pub P1::Value, pub > as Pattern>::Value) + where P1: Pattern; + From 913937418150f79d80c78f1c69846903f7aaeaeb Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 28 Nov 2018 21:39:20 -0500 Subject: [PATCH 4/4] Fix Tidy error --- src/test/rustdoc/synthetic_auto/self-referential.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/rustdoc/synthetic_auto/self-referential.rs b/src/test/rustdoc/synthetic_auto/self-referential.rs index 077786b280fc3..516a3c9a516ab 100644 --- a/src/test/rustdoc/synthetic_auto/self-referential.rs +++ b/src/test/rustdoc/synthetic_auto/self-referential.rs @@ -35,6 +35,6 @@ impl Pattern for Wrapper { // @has self_referential/struct.WriteAndThen.html // @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl Send for \ // WriteAndThen where ::Value: Send" -pub struct WriteAndThen(pub P1::Value, pub > as Pattern>::Value) +pub struct WriteAndThen(pub P1::Value,pub > as Pattern>::Value) where P1: Pattern;