Skip to content

Ensure that Rustdoc discovers all necessary auto trait bounds #55318

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

Merged
merged 4 commits into from
Dec 7, 2018
Merged
Changes from 1 commit
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
Next Next commit
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.
Aaron1011 committed Nov 28, 2018

Verified

This commit was signed with the committer’s verified signature.
Aaron1011 Aaron Hill
commit f57247c48cb59a59dcfcb220251206064265479c
68 changes: 51 additions & 17 deletions src/librustc/traits/auto_trait.rs
Original file line number Diff line number Diff line change
@@ -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 {
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 {
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!(
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,
}