Skip to content

Commit

Permalink
Allow supertrait edge to resolve to common built-in traits too. (#464)
Browse files Browse the repository at this point in the history
  • Loading branch information
obi1kenobi committed Sep 8, 2024
1 parent d840859 commit da86e1a
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 33 deletions.
24 changes: 21 additions & 3 deletions src/adapter/edges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,9 +558,27 @@ pub(super) fn resolve_trait_edge<'a, V: AsVertex<Vertex<'a>> + 'a>(
let trait_vertex = vertex.as_trait().expect("not a Trait vertex");
Box::new(trait_vertex.bounds.iter().filter_map(move |bound| {
if let TraitBound { trait_, .. } = &bound {
item_index
.get(&trait_.id)
.as_ref()
// When the implemented trait is from the same crate
// as its definition, the trait is expected to be present
// in `item_index`. Otherwise, the
// `rustdoc_types::Trait` is not in this rustdoc,
// even if the trait is part of Rust `core` or `std`.
// As a temporary workaround, some common
// Rust built-in traits are manually "inlined"
// with items stored in `manually_inlined_builtin_traits`.
let found_item = item_index.get(&trait_.id).or_else(|| {
let manually_inlined_builtin_traits = match origin {
Origin::CurrentCrate => &current_crate.manually_inlined_builtin_traits,
Origin::PreviousCrate => {
&previous_crate
.expect("no previous crate provided")
.manually_inlined_builtin_traits
}
};
manually_inlined_builtin_traits.get(&trait_.id)
});

found_item
.map(|next_item| origin.make_implemented_trait_vertex(trait_, next_item))
} else {
None
Expand Down
5 changes: 3 additions & 2 deletions src/adapter/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,10 +487,11 @@ pub(super) fn resolve_implemented_trait_property<'a, V: AsVertex<Vertex<'a>> + '
) -> ContextOutcomeIterator<'a, V, FieldValue> {
match property_name {
"name" => resolve_property_with(contexts, |vertex| {
let (path, _) = vertex
let (_, item) = vertex
.as_implemented_trait()
.expect("not an ImplementedTrait");
path.name.clone().into()

item.name.clone().into()
}),
_ => unreachable!("ImplementedTrait property {property_name}"),
}
Expand Down
22 changes: 19 additions & 3 deletions src/adapter/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,10 @@ fn rustdoc_finds_supertrait() {
Crate {
item {
... on Trait {
name @output
supertrait {
name @output
supertrait: name @output
}
}
}
Expand All @@ -152,6 +154,7 @@ fn rustdoc_finds_supertrait() {
#[derive(Debug, PartialOrd, Ord, PartialEq, Eq, serde::Deserialize)]
struct Output {
name: String,
supertrait: String,
}

let mut results: Vec<_> =
Expand All @@ -164,10 +167,23 @@ fn rustdoc_finds_supertrait() {
similar_asserts::assert_eq!(
vec![
Output {
name: "Supertrait".into(),
name: "DebugPartialOrd".into(),
// We *specifically* require the supertrait name to be "Debug",
// not "std::fmt::Debug" or any other option. Failing to do this
// could cause false-positives in cargo-semver-checks.
supertrait: "Debug".into(),
},
Output {
name: "DebugPartialOrd".into(),
supertrait: "PartialOrd".into(),
},
Output {
name: "MyTrait".into(),
supertrait: "Supertrait".into(),
},
Output {
name: "Supertrait2".into(),
name: "MyTrait".into(),
supertrait: "Supertrait2".into(),
},
],
results
Expand Down
64 changes: 40 additions & 24 deletions src/indexed_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ impl<'a> IndexedCrate<'a> {
impl_index: None,
};

debug_assert!(
!value.manually_inlined_builtin_traits.is_empty(),
"failed to find any traits to manually inline",
);

// Build the imports index
//
// This is inlined because we need access to `value`, but `value` is not a valid
Expand Down Expand Up @@ -413,6 +418,7 @@ impl<'a: 'b, 'b> Borrow<(&'b Id, &'b str)> for ImplEntry<'a> {
#[derive(Debug)]
struct ManualTraitItem {
name: &'static str,
path: &'static [&'static str],
is_auto: bool,
is_unsafe: bool,
}
Expand All @@ -423,71 +429,85 @@ struct ManualTraitItem {
const MANUAL_TRAIT_ITEMS: [ManualTraitItem; 14] = [
ManualTraitItem {
name: "Debug",
path: &["core", "fmt", "Debug"],
is_auto: false,
is_unsafe: false,
},
ManualTraitItem {
name: "Clone",
path: &["core", "clone", "Clone"],
is_auto: false,
is_unsafe: false,
},
ManualTraitItem {
name: "Copy",
path: &["core", "marker", "Copy"],
is_auto: false,
is_unsafe: false,
},
ManualTraitItem {
name: "PartialOrd",
path: &["core", "cmp", "PartialOrd"],
is_auto: false,
is_unsafe: false,
},
ManualTraitItem {
name: "Ord",
path: &["core", "cmp", "Ord"],
is_auto: false,
is_unsafe: false,
},
ManualTraitItem {
name: "PartialEq",
path: &["core", "cmp", "PartialEq"],
is_auto: false,
is_unsafe: false,
},
ManualTraitItem {
name: "Eq",
path: &["core", "cmp", "Eq"],
is_auto: false,
is_unsafe: false,
},
ManualTraitItem {
name: "Hash",
path: &["core", "hash", "Hash"],
is_auto: false,
is_unsafe: false,
},
ManualTraitItem {
name: "Send",
path: &["core", "marker", "Send"],
is_auto: true,
is_unsafe: true,
},
ManualTraitItem {
name: "Sync",
path: &["core", "marker", "Sync"],
is_auto: true,
is_unsafe: true,
},
ManualTraitItem {
name: "Unpin",
path: &["core", "marker", "Unpin"],
is_auto: true,
is_unsafe: false,
},
ManualTraitItem {
name: "RefUnwindSafe",
path: &["core", "panic", "unwind_safe", "RefUnwindSafe"],
is_auto: true,
is_unsafe: false,
},
ManualTraitItem {
name: "UnwindSafe",
path: &["core", "panic", "unwind_safe", "UnwindSafe"],
is_auto: true,
is_unsafe: false,
},
ManualTraitItem {
name: "Sized",
path: &["core", "marker", "Sized"],
is_auto: false,
is_unsafe: false,
},
Expand Down Expand Up @@ -534,31 +554,27 @@ fn new_trait(manual_trait_item: &ManualTraitItem, id: Id, crate_id: u32) -> Item
}

fn create_manually_inlined_builtin_traits(crate_: &Crate) -> HashMap<Id, Item> {
let paths = crate_
.index
.values()
.map(|item| &item.inner)
.filter_map(|item_enum| match item_enum {
rustdoc_types::ItemEnum::Impl(impl_) => Some(impl_),
_ => None,
})
.filter_map(|impl_| impl_.trait_.as_ref());
let paths = &crate_.paths;

paths
.filter_map(|path| {
MANUAL_TRAIT_ITEMS
.iter()
.find(|manual| manual.name == path.name)
.and_then(|manual| {
crate_.paths.get(&path.id).map(|item_summary| {
(
path.id.clone(),
new_trait(manual, path.id.clone(), item_summary.crate_id),
)
})
})
})
.collect()
// `paths` may have thousands of items.
#[cfg(feature = "rayon")]
let iter = paths.par_iter();
#[cfg(not(feature = "rayon"))]
let iter = paths.iter();

iter.filter_map(|(id, entry)| {
if entry.kind != rustdoc_types::ItemKind::Trait {
return None;
}

// This is a linear scan, but across a tiny array.
// It isn't worth doing anything fancier here.
MANUAL_TRAIT_ITEMS
.iter()
.find(|t| t.path == entry.path)
.map(|manual| (id.clone(), new_trait(manual, id.clone(), entry.crate_id)))
})
.collect()
}

#[cfg(test)]
Expand Down
7 changes: 6 additions & 1 deletion test_crates/supertrait/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,9 @@ pub trait Supertrait {}

pub trait Supertrait2 {}

pub trait BaseTrait : Supertrait2 + Supertrait {}
pub trait MyTrait : Supertrait2 + Supertrait {}

// Ensure `std::fmt::Debug` is a supertrait here, verbatim.
// It's a load-bearing part of the test, since we want our query output names to be invariant
// to how they are written down in the source.
pub trait DebugPartialOrd : std::fmt::Debug + PartialOrd {}

0 comments on commit da86e1a

Please sign in to comment.