Fix primitive search in parameters and returned values #81557
Conversation
rust-highfive
added
the
S-waiting-on-review
camelid
added
A-type-based-search
GuillaumeGomez
force-pushed
the
primitive-search
branch
from
74d8563
to
00a4c76
Compare
|
Updated! |
This comment has been minimized.
This comment has been minimized.
GuillaumeGomez
force-pushed
the
primitive-search
branch
from
00a4c76
to
f2ce213
Compare
|
Btw, I'm not sure if ollie27 is still doing reviews. (Correct me if I'm wrong Ollie! I just want to make sure in case you don't have time to do reviews right now :) |
|
Looks good. @bors r+ |
|
📌 Commit f2ce21341621d04cdf689652b73bfb5de403120e has been approved by |
bors
added
S-waiting-on-bors
|
Actually this isn't quite right @bors r- |
bors
added
S-waiting-on-author
| if let Some(kind) = ty.def_id().map(|did| cx.tcx.def_kind(did).clean(cx)) { | ||
| res.insert((ty, kind)); | ||
| } else { | ||
| // This is a primitive, let's store it as such. |
There was a problem hiding this comment.
This isn't always a primitive. ty.def_id() will return None for generic parameters as well which shouldn't end up the in the search index.
There was a problem hiding this comment.
Great catch! I'll fix that and add a test to ensure they're not listed.
There was a problem hiding this comment.
Actually no because we check before if it's a full generic. So it should never be the case. I'll add a test to ensure it though.
There was a problem hiding this comment.
is_full_generic will only return true for generics passed by value. Generics passed by reference like pub fn foo<T>(x: &T) {} or &self show up in the search index with this PR.
There was a problem hiding this comment.
I see. I'll add a is_primitive method then to ensure that.
GuillaumeGomez
force-pushed
the
primitive-search
branch
from
f2ce213
to
92bb9bc
Compare
|
@ollie27 I extended the test to ensure that generics are not added into the search index. |
GuillaumeGomez
force-pushed
the
primitive-search
branch
from
92bb9bc
to
cbb03bb
Compare
GuillaumeGomez
force-pushed
the
primitive-search
branch
from
cbb03bb
to
c013f2a
Compare
|
@ollie27 Re-updated the test to test generics behind reference and added the |
|
Yep, that looks better @bors r+ |
|
📌 Commit c013f2a has been approved by |
bors
added
S-waiting-on-bors
|
Thanks for the improvement! :) |
…ollie27 Fix primitive search in parameters and returned values Part of rust-lang#60485. Fixes rust-lang#74780. Replacing rust-lang#74879. cc `@camelid` `@jyn514` `@CraftSpider` r? `@ollie27`
| @@ -78,7 +78,7 @@ crate fn build_index(krate: &clean::Crate, cache: &mut Cache) -> String { | |||
| desc: item.doc_value().map_or_else(String::new, |s| short_markdown_summary(&s)), | |||
| parent: Some(did), | |||
| parent_idx: None, | |||
| search_type: get_index_search_type(&item, None), | |||
| search_type: get_index_search_type(&item, Some(cache)), | |||
There was a problem hiding this comment.
Just to make sure I understand what is happening here: This adds primitives to the cache, because now in an earlier stage they are being properly added to the cache via the new insert method? And previously this wasn't used because... Cache wasn't working right on them, I'm guessing?
There was a problem hiding this comment.
Exactly. To be more precise: the cache here is needed to get the DefId of primitive types. If we don't have primitives, we don't need cache (and before that we didn't get the primitives in the cache). So you got it exactly rght. :)
|
☀️ Test successful - checks-actions |
Part of #60485.
Fixes #74780.
Replacing #74879.
cc @camelid @jyn514 @CraftSpider
r? @ollie27