From bff08f2731fe658d50e291e8b6285e9cd735b528 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 22 Jun 2018 22:25:56 -0400 Subject: [PATCH 1/4] Fix rustdoc crash when 'static bound appears in struct declaration --- src/librustdoc/clean/auto_trait.rs | 4 ++-- .../rustdoc/synthetic_auto/static-region.rs | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 src/test/rustdoc/synthetic_auto/static-region.rs diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs index c30d6817b46..69f2ac5bd66 100644 --- a/src/librustdoc/clean/auto_trait.rs +++ b/src/librustdoc/clean/auto_trait.rs @@ -642,8 +642,8 @@ impl<'a, 'tcx, 'rcx> AutoTraitFinder<'a, 'tcx, 'rcx> { name: name.to_string(), kind: GenericParamDefKind::Lifetime, }) - } - &ty::ReVar(_) | &ty::ReEarlyBound(_) => None, + }, + &ty::ReVar(_) | &ty::ReEarlyBound(_) | &ty::ReStatic => None, _ => panic!("Unexpected region type {:?}", r), } }) diff --git a/src/test/rustdoc/synthetic_auto/static-region.rs b/src/test/rustdoc/synthetic_auto/static-region.rs new file mode 100644 index 00000000000..96e8b8ed5f6 --- /dev/null +++ b/src/test/rustdoc/synthetic_auto/static-region.rs @@ -0,0 +1,20 @@ +// 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 OwnedTrait<'a> { + type Reader; +} + +// @has static_region/struct.Owned.html +// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl Send for \ +// Owned where >::Reader: Send" +pub struct Owned where T: OwnedTrait<'static> { + marker: >::Reader, +} From a0943b6bba4ed2ba6f32635fecef794325b5fdf4 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 2 Aug 2018 13:59:16 -0400 Subject: [PATCH 2/4] Filter out duplicated trait predicates when generating auto traits Fixes #51236 --- src/librustc/traits/auto_trait.rs | 66 +++++++++++++++++++++++++++++-- src/test/rustdoc/issue-51236.rs | 24 +++++++++++ 2 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 src/test/rustdoc/issue-51236.rs diff --git a/src/librustc/traits/auto_trait.rs b/src/librustc/traits/auto_trait.rs index f4873979920..ffc4838cf8b 100644 --- a/src/librustc/traits/auto_trait.rs +++ b/src/librustc/traits/auto_trait.rs @@ -326,6 +326,9 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { let mut user_computed_preds: FxHashSet<_> = user_env.caller_bounds.iter().cloned().collect(); + + + let mut new_env = param_env.clone(); let dummy_cause = ObligationCause::misc(DUMMY_SP, ast::DUMMY_NODE_ID); @@ -358,7 +361,8 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { &Err(SelectionError::Unimplemented) => { if self.is_of_param(pred.skip_binder().trait_ref.substs) { already_visited.remove(&pred); - user_computed_preds.insert(ty::Predicate::Trait(pred.clone())); + self.add_user_pred(&mut user_computed_preds, ty::Predicate::Trait(pred.clone())); + //user_computed_preds.insert(ty::Predicate::Trait(pred.clone())); predicates.push_back(pred); } else { debug!( @@ -393,6 +397,62 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { return Some((new_env, final_user_env)); } + fn add_user_pred<'c>(&self, user_computed_preds: &mut FxHashSet>, new_pred: ty::Predicate<'c>) { + let mut should_add_new = true; + user_computed_preds.retain(|&old_pred| { + match (&new_pred, old_pred) { + (&ty::Predicate::Trait(new_trait), ty::Predicate::Trait(old_trait)) => { + if new_trait.def_id() == old_trait.def_id() { + let new_substs = new_trait.skip_binder().trait_ref.substs; + let old_substs = old_trait.skip_binder().trait_ref.substs; + if !new_substs.types().eq(old_substs.types()) { + // We can't compare lifetimes if the types are different, + // so skip checking old_pred + return true + } + + for (new_region, old_region) in new_substs.regions().zip(old_substs.regions()) { + match (new_region, old_region) { + // If both predicates have an 'ReLateBound' (a HRTB) in the + // same spot, we do nothing + (ty::RegionKind::ReLateBound(_, _), ty::RegionKind::ReLateBound(_, _)) => {}, + + (ty::RegionKind::ReLateBound(_, _), _) => { + // The new predicate has a HRTB in a spot where the old + // predicate does not (if they both had a HRTB, the previous + // match arm would have executed). + // + // The means we want to remove the older predicate from + // user_computed_preds, since having both it and the new + // predicate in a ParamEnv would confuse SelectionContext + // We're currently in the predicate passed to 'retain', + // so we return 'false' to remove the old predicate from + // user_computed_preds + return false; + }, + (_, ty::RegionKind::ReLateBound(_, _)) => { + // This is the opposite situation as the previous arm - the + // old predicate has a HRTB lifetime in a place where the + // new predicate does not. We want to leave the old + // predicate in user_computed_preds, and skip adding + // new_pred to user_computed_params. + should_add_new = false + } + _ => {} + } + } + } + }, + _ => {} + } + return true + }); + + if should_add_new { + user_computed_preds.insert(new_pred); + } + } + pub fn region_name(&self, region: Region) -> Option { match region { &ty::ReEarlyBound(r) => Some(r.name.to_string()), @@ -555,7 +615,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { let substs = &p.skip_binder().trait_ref.substs; if self.is_of_param(substs) && !only_projections && is_new_pred { - computed_preds.insert(predicate); + self.add_user_pred(computed_preds, predicate); } predicates.push_back(p.clone()); } @@ -563,7 +623,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { // 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 { - computed_preds.insert(predicate); + self.add_user_pred(computed_preds, predicate); } else { match poly_project_and_unify_type(select, &obligation.with(p.clone())) { Err(e) => { diff --git a/src/test/rustdoc/issue-51236.rs b/src/test/rustdoc/issue-51236.rs new file mode 100644 index 00000000000..541a1c5e19f --- /dev/null +++ b/src/test/rustdoc/issue-51236.rs @@ -0,0 +1,24 @@ +// 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. + +use std::marker::PhantomData; + +pub mod traits { + pub trait Owned<'a> { + type Reader; + } +} + +// @has issue_51236/struct.Owned.html +// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl Send for \ +// Owned where >::Reader: Send" +pub struct Owned where T: for<'a> ::traits::Owned<'a> { + marker: PhantomData<>::Reader>, +} From 016b5866852e3d9c7c5ba6cdc3938afe7976af9e Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 2 Aug 2018 14:34:25 -0400 Subject: [PATCH 3/4] A bit of cleanup --- src/librustc/traits/auto_trait.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/librustc/traits/auto_trait.rs b/src/librustc/traits/auto_trait.rs index ffc4838cf8b..ea96bd86f92 100644 --- a/src/librustc/traits/auto_trait.rs +++ b/src/librustc/traits/auto_trait.rs @@ -326,9 +326,6 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { let mut user_computed_preds: FxHashSet<_> = user_env.caller_bounds.iter().cloned().collect(); - - - let mut new_env = param_env.clone(); let dummy_cause = ObligationCause::misc(DUMMY_SP, ast::DUMMY_NODE_ID); @@ -362,7 +359,6 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { if self.is_of_param(pred.skip_binder().trait_ref.substs) { already_visited.remove(&pred); self.add_user_pred(&mut user_computed_preds, ty::Predicate::Trait(pred.clone())); - //user_computed_preds.insert(ty::Predicate::Trait(pred.clone())); predicates.push_back(pred); } else { debug!( @@ -397,6 +393,29 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { return Some((new_env, final_user_env)); } + // This method is designed to work around the following issue: + // When we compute auto trait bounds, we repeatedly call SelectionContext.select, + // progressively building a ParamEnv based on the results we get. + // However, our usage of SelectionContext differs from its normal use within the compiler, + // in that we capture and re-reprocess predicates from Unimplemented errors. + // + // This can lead to a corner case when dealing with region parameters. + // During our selection loop in evaluate_predicates, we might end up with + // two trait predicates that differ only in their region parameters: + // one containing a HRTB lifetime parameter, and one containing a 'normal' + // lifetime parameter. For example: + // + // T as MyTrait<'a> + // T as MyTrait<'static> + // + // If we put both of these predicates in our computed ParamEnv, we'll + // confuse SelectionContext, since it will (correctly) view both as being applicable. + // + // To solve this, we pick the 'more strict' lifetime bound - i.e. the HRTB + // Our end goal is to generate a user-visible description of the conditions + // under which a type implements an auto trait. A trait predicate involving + // a HRTB means that the type needs to work with any choice of lifetime, + // not just one specific lifetime (e.g. 'static). fn add_user_pred<'c>(&self, user_computed_preds: &mut FxHashSet>, new_pred: ty::Predicate<'c>) { let mut should_add_new = true; user_computed_preds.retain(|&old_pred| { From b010d1f9295821f2f37de76a84ea4e26620456dd Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 2 Aug 2018 14:58:13 -0400 Subject: [PATCH 4/4] Tidy fixes --- src/librustc/traits/auto_trait.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/librustc/traits/auto_trait.rs b/src/librustc/traits/auto_trait.rs index ea96bd86f92..cfe3583a980 100644 --- a/src/librustc/traits/auto_trait.rs +++ b/src/librustc/traits/auto_trait.rs @@ -358,7 +358,8 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { &Err(SelectionError::Unimplemented) => { if self.is_of_param(pred.skip_binder().trait_ref.substs) { already_visited.remove(&pred); - self.add_user_pred(&mut user_computed_preds, ty::Predicate::Trait(pred.clone())); + self.add_user_pred(&mut user_computed_preds, + ty::Predicate::Trait(pred.clone())); predicates.push_back(pred); } else { debug!( @@ -416,7 +417,8 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { // under which a type implements an auto trait. A trait predicate involving // a HRTB means that the type needs to work with any choice of lifetime, // not just one specific lifetime (e.g. 'static). - fn add_user_pred<'c>(&self, user_computed_preds: &mut FxHashSet>, new_pred: ty::Predicate<'c>) { + fn add_user_pred<'c>(&self, user_computed_preds: &mut FxHashSet>, + new_pred: ty::Predicate<'c>) { let mut should_add_new = true; user_computed_preds.retain(|&old_pred| { match (&new_pred, old_pred) { @@ -430,11 +432,17 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { return true } - for (new_region, old_region) in new_substs.regions().zip(old_substs.regions()) { + for (new_region, old_region) in new_substs + .regions() + .zip(old_substs.regions()) { + match (new_region, old_region) { // If both predicates have an 'ReLateBound' (a HRTB) in the // same spot, we do nothing - (ty::RegionKind::ReLateBound(_, _), ty::RegionKind::ReLateBound(_, _)) => {}, + ( + ty::RegionKind::ReLateBound(_, _), + ty::RegionKind::ReLateBound(_, _) + ) => {}, (ty::RegionKind::ReLateBound(_, _), _) => { // The new predicate has a HRTB in a spot where the old