Skip to content

Fix invalid bounds string generation in rustdoc #58894

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.

Sign up for GitHub

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 3 commits into from
Apr 6, 2019
Merged

Fix invalid bounds string generation in rustdoc #58894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ddd034a
Fix invalid bounds string generation in rustdoc
GuillaumeGomez Mar 31, 2019
f6eb09b
Fix explicit_predicates_of
GuillaumeGomez Apr 3, 2019
c966c45
Add test from #59033
GuillaumeGomez Apr 3, 2019
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
5 changes: 4 additions & 1 deletion src/librustc_typeck/collect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1918,7 +1918,10 @@ fn explicit_predicates_of<'a, 'tcx>(
}
}

let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap();
let hir_id = match tcx.hir().as_local_hir_id(def_id) {
Some(hir_id) => hir_id,
None => return tcx.predicates_of(def_id),
Copy link
Member

Choose a reason for hiding this comment

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

This is another change to rustc_typeck that wasn't explicitly approved (the comment thread doesn't make it clear that a hack was added outside of rustdoc), can we please not do this? I'll create a PR reverting these, but I feel like we have a larger process break issue...

Yes, I didn't see this notification until now, but please ping me elsewhere instead of merging typeck hacks like this.

Copy link
Member

Choose a reason for hiding this comment

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

Not to mention this is wrong for cross-crate items, my expectation was that explicit_predicates_of was available cross-crate.

Copy link
Contributor

Choose a reason for hiding this comment

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

@estebank ^

Copy link
Member Author

Choose a reason for hiding this comment

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

I did ping the compiler team so I thought you'd see the notification, my bad.

Copy link
Contributor

Choose a reason for hiding this comment

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

Talked with Eddy on chat. My bad as the previous seemed to make sense on isolation. I'll defer to Eddy's assessment on typeck code in the future.

Copy link
Member

Choose a reason for hiding this comment

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

@GuillaumeGomez Being notified isn't enough, we need to acknowledge it too.
With @nikomatsakis less active lately (and also on PTO more recently), a bit more care should be taken.

Copy link
Member

Choose a reason for hiding this comment

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

Opened #59789.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't approved the PR nor asked anyone (except the compiler team) to review it. I approve for more care to be taken but I feel like taking the blame for an error I didn't make. :-/

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a process failure, not your failure! We'll figure out how to prevent such things happening in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

@GuillaumeGomez The one at fault here was me, not you, as I approved this change thinking it looked reasonable without being my main area of expertise, I should have deferred to @eddyb's or @nikomatsakis' judgement on this case.

};
let node = tcx.hir().get_by_hir_id(hir_id);

let mut is_trait = None;
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/auto_trait.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
(replaced.clone(), replaced.clean(self.cx))
});

let full_generics = (&type_generics, &tcx.predicates_of(did));
let full_generics = (&type_generics, &tcx.explicit_predicates_of(did));
let Generics {
params: mut generic_params,
..
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/blanket_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
.collect();

let ty = self.cx.get_real_ty(def_id, def_ctor, &real_name, generics);
let predicates = infcx.tcx.predicates_of(impl_def_id);
let predicates = infcx.tcx.explicit_predicates_of(impl_def_id);

impls.push(Item {
source: infcx.tcx.def_span(impl_def_id).clean(self.cx),
Expand Down
10 changes: 5 additions & 5 deletions src/librustdoc/clean/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ fn build_external_function(cx: &DocContext<'_>, did: DefId) -> clean::Function {
}

fn build_enum(cx: &DocContext<'_>, did: DefId) -> clean::Enum {
let predicates = cx.tcx.predicates_of(did);
let predicates = cx.tcx.explicit_predicates_of(did);

clean::Enum {
generics: (cx.tcx.generics_of(did), &predicates).clean(cx),
Expand All @@ -238,7 +238,7 @@ fn build_enum(cx: &DocContext<'_>, did: DefId) -> clean::Enum {
}

fn build_struct(cx: &DocContext<'_>, did: DefId) -> clean::Struct {
let predicates = cx.tcx.predicates_of(did);
let predicates = cx.tcx.explicit_predicates_of(did);
let variant = cx.tcx.adt_def(did).non_enum_variant();

clean::Struct {
Expand All @@ -254,7 +254,7 @@ fn build_struct(cx: &DocContext<'_>, did: DefId) -> clean::Struct {
}

fn build_union(cx: &DocContext<'_>, did: DefId) -> clean::Union {
let predicates = cx.tcx.predicates_of(did);
let predicates = cx.tcx.explicit_predicates_of(did);
let variant = cx.tcx.adt_def(did).non_enum_variant();

clean::Union {
Expand All @@ -266,7 +266,7 @@ fn build_union(cx: &DocContext<'_>, did: DefId) -> clean::Union {
}

fn build_type_alias(cx: &DocContext<'_>, did: DefId) -> clean::Typedef {
let predicates = cx.tcx.predicates_of(did);
let predicates = cx.tcx.explicit_predicates_of(did);

clean::Typedef {
type_: cx.tcx.type_of(did).clean(cx),
Expand Down Expand Up @@ -325,7 +325,7 @@ pub fn build_impl(cx: &DocContext<'_>, did: DefId, ret: &mut Vec<clean::Item>) {
}
}

let predicates = tcx.predicates_of(did);
let predicates = tcx.explicit_predicates_of(did);
let (trait_items, generics) = if let Some(hir_id) = tcx.hir().as_local_hir_id(did) {
match tcx.hir().expect_item_by_hir_id(hir_id).node {
hir::ItemKind::Impl(.., ref gen, _, _, ref item_ids) => {
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2281,7 +2281,7 @@ impl<'tcx> Clean<Item> for ty::AssociatedItem {
}
ty::AssociatedKind::Method => {
let generics = (cx.tcx.generics_of(self.def_id),
&cx.tcx.predicates_of(self.def_id)).clean(cx);
&cx.tcx.explicit_predicates_of(self.def_id)).clean(cx);
let sig = cx.tcx.fn_sig(self.def_id);
let mut decl = (self.def_id, sig).clean(cx);

Expand Down Expand Up @@ -2354,7 +2354,7 @@ impl<'tcx> Clean<Item> for ty::AssociatedItem {
// are actually located on the trait/impl itself, so we need to load
// all of the generics from there and then look for bounds that are
// applied to this associated type in question.
let predicates = cx.tcx.predicates_of(did);
let predicates = cx.tcx.explicit_predicates_of(did);
let generics = (cx.tcx.generics_of(did), &predicates).clean(cx);
let mut bounds = generics.where_predicates.iter().filter_map(|pred| {
let (name, self_type, trait_, bounds) = match *pred {
Expand Down Expand Up @@ -3062,7 +3062,7 @@ impl<'tcx> Clean<Type> for Ty<'tcx> {
ty::Opaque(def_id, substs) => {
// Grab the "TraitA + TraitB" from `impl TraitA + TraitB`,
// by looking up the projections associated with the def_id.
let predicates_of = cx.tcx.predicates_of(def_id);
let predicates_of = cx.tcx.explicit_predicates_of(def_id);
let substs = cx.tcx.lift(&substs).expect("Opaque lift failed");
let bounds = predicates_of.instantiate(cx.tcx, substs);
let mut regions = vec![];
Expand Down
13 changes: 13 additions & 0 deletions src/test/rustdoc/useless_lifetime_bound.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use std::marker::PhantomData;

// @has useless_lifetime_bound/struct.Scope.html
// @!has - '//*[@class="rust struct"]' "'env: 'env"
pub struct Scope<'env> {
_marker: PhantomData<&'env mut &'env ()>,
}

// @has useless_lifetime_bound/struct.Scope.html
// @!has - '//*[@class="rust struct"]' "T: 'a + 'a"
pub struct SomeStruct<'a, T: 'a> {
_marker: PhantomData<&'a T>,
}