rustdoc: Start cleaning up search index generation #92340
Conversation
The old name wasn't very clear, while the new one makes it clear that this is the code responsible for creating the search index.
Based on rust-lang#80883 (comment). The `tcx` parameters do seem to be used though, so I only removed the `cache` parameters.
It was previously defined in `render::search_index` but wasn't used at all there. `clean::types` seems like a better fit since that's where `ExternalCrate` is defined.
Now the only two crate-public items are `build_index` and `get_index_search_type` (because for some reason the latter is also used in `formats::cache`).
camelid
added
A-rustdoc-search
|
Some changes occurred in cc @camelid |
rustbot
added
the
T-rustdoc
rust-highfive
added
the
S-waiting-on-review
|
I recommend reviewing this commit-by-commit. |
| ) { | ||
| fn insert_ty( | ||
| res: &mut Vec<TypeWithKind>, | ||
| tcx: TyCtxt<'_>, | ||
| ty: Type, | ||
| mut generics: Vec<TypeWithKind>, | ||
| _cache: &Cache, |
There was a problem hiding this comment.
I don't think this is the right move because of:
} else if let Some(kind) = ty.def_id_no_primitives().map(|did| tcx.def_kind(did).into()) {We are replacing def_id_no_primitives calls, so we need to keep the Cache.
There was a problem hiding this comment.
Hmm, I see what you mean, but the Cache can easily be re-added later. As of now, passing around the Cache is just dead code that might be sitting there for a while.
There was a problem hiding this comment.
I reviewed a PR doing exactly that today (but of course, can't find it again...). I'd prefer to avoid having removals/adds following but not sure it matters much since it's just about git history here...
There was a problem hiding this comment.
I think you're thinking of #92342? I'd rather remove the dead code for now – e.g., that PR may not work and end up being closed, and then the dead code will still be there.
There was a problem hiding this comment.
I don't fully agree with you but it's fine. Let's move forward then!
| @@ -378,10 +378,12 @@ fn get_real_types<'tcx>( | |||
| /// i.e. `fn foo<A: Display, B: Option<A>>(x: u32, y: B)` will return | |||
| /// `[u32, Display, Option]`. | |||
| fn get_all_types<'tcx>( | |||
| generics: &Generics, | |||
| decl: &FnDecl, | |||
| func: &Function, | |||
| clean::BorrowedRef { ref type_, .. } => get_index_type_name(type_, accept_generic), | ||
| clean::Generic(_) | ||
| | clean::BareFunction(_) | ||
| clean::BorrowedRef { ref type_, .. } => get_index_type_name(type_), |
|
Overall, this is really great, thanks! Just one change that would be problematic and I think it'd be better to remove it. |
camelid
force-pushed
the
search-index-cleanup
branch
from
132d2f4
to
a6ea35c
Compare
camelid
force-pushed
the
search-index-cleanup
branch
from
a6ea35c
to
cf398b8
Compare
|
r=me once CI pass. |
This comment has been minimized.
This comment has been minimized.
This issue was fixed using a hacky recursion "fuel" argument, but the issue was never minimized nor was a regression test added. The underlying bug is still unfixed, so this test should help with fixing it and removing the `recurse` hack.
camelid
force-pushed
the
search-index-cleanup
branch
from
cf398b8
to
908a9d4
Compare
|
@bors r=GuillaumeGomez |
|
📌 Commit 908a9d4 has been approved by |
bors
added
S-waiting-on-bors
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#92075 (rustdoc: Only special case struct fields for intra-doc links, not enum variants) - rust-lang#92118 (Parse and suggest moving where clauses after equals for type aliases) - rust-lang#92237 (Visit expressions in-order when resolving pattern bindings) - rust-lang#92340 (rustdoc: Start cleaning up search index generation) - rust-lang#92351 (Add long error explanation for E0227) - rust-lang#92371 (Remove pretty printer space inside block with only outer attrs) - rust-lang#92372 (Print space after formal generic params in fn type) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
I'm trying to simplify and clean up the code, partly to make #90779 easier.
r? @GuillaumeGomez