Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that Rustdoc discovers all necessary auto trait bounds #55318

Merged
merged 4 commits into from
Dec 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 81 additions & 17 deletions src/librustc/traits/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)) => {
Expand Down Expand Up @@ -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_param_no_infer(pred.skip_binder().trait_ref.substs) {
already_visited.remove(&pred);
self.add_user_pred(
&mut user_computed_preds,
Expand Down Expand Up @@ -631,18 +636,28 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
finished_map
}

pub fn is_of_param(&self, substs: &Substs<'_>) -> bool {
if substs.is_noop() {
return false;
}
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());
}

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,
};
}

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,
Expand All @@ -661,28 +676,77 @@ 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_param_no_infer(p.skip_binder().trait_ref.substs)
&& !only_projections
&& is_new_pred {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we check for inference variables for the other parts of p?

Copy link
Member Author

Choose a reason for hiding this comment

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

That was an oversight on my part - I intended to check all of the parameters in substs, but forgot to change it. I've pushed a new commit fixing it.


if self.is_of_param(substs) && !only_projections && is_new_pred {
self.add_user_pred(computed_preds, predicate);
}
predicates.push_back(p.clone());
Copy link
Contributor

Choose a reason for hiding this comment

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

Pre-existing, but can you explain to me why this adds a user-predicate (above) and then still pushes p back onto the list to be selected again?

Copy link
Member Author

Choose a reason for hiding this comment

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

Selecting p might result in other predicates (e.g. TypeOutlives or RegionOutlives) that are necessary for FulfillmentContext.select_all_or_error to succeed.

In general, AutoTraitFinder always tries to drive select all the way to the end. A subset of the predicates that it finds along the way are suitable for displaying to a user (e.g. rendering in docs), which we track separately.

}
&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 {
self.add_user_pred(computed_preds, predicate);
} else {
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_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\
to computed_preds: {:?}", predicate);

// Under unusual circumstances, we can end up with a self-refeential
// projection predicate. For example:
// <T as MyType>::Value == <T as MyType>::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
// 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!(
Expand Down
31 changes: 31 additions & 0 deletions src/test/rustdoc/issue-50159.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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<B, C> Signal2 for B where B: Signal<Item = C> {
type Item2 = C;
}

// @has issue_50159/struct.Switch.html
// @has - '//code' 'impl<B> Send for Switch<B> where <B as Signal>::Item: Send'
// @has - '//code' 'impl<B> Sync for Switch<B> where <B as Signal>::Item: Sync'
// @count - '//*[@id="implementations-list"]/*[@class="impl"]' 0
// @count - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]' 2
pub struct Switch<B: Signal> {
pub inner: <B as Signal2>::Item2,
}
40 changes: 40 additions & 0 deletions src/test/rustdoc/synthetic_auto/self-referential.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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 = A, C = A>(A, B, C);

impl<A, B, C> Pattern for Constrain<A, B, C>
where A: Pattern,
B: Pattern<Value = A::Value>,
C: Pattern<Value = A::Value>,
{
type Value = A::Value;
}

pub struct Wrapper<T>(T);

impl<T> Pattern for Wrapper<T> {
type Value = T;
}


// @has self_referential/struct.WriteAndThen.html
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<P1> Send for \
// WriteAndThen<P1> where <P1 as Pattern>::Value: Send"
pub struct WriteAndThen<P1>(pub P1::Value,pub <Constrain<P1, Wrapper<P1::Value>> as Pattern>::Value)
where P1: Pattern;