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

Allow supertrait edge to resolve to common built-in traits too. #464

Merged
merged 1 commit into from
Sep 8, 2024
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
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 {}