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

Clean up ImportMap #15181

Merged
merged 5 commits into from Jul 3, 2023
Merged

Conversation

lowr
Copy link
Contributor

@lowr lowr commented Jun 30, 2023

There are several things in hir_def::import_map that are never used. This PR removes them and restructures the code. Namely:

  • Removes Query::name_only, because it's always true.
    • Because of this, we never took advantage of storing items' full path. This PR removes ImportPath and changes ImportInfo to only store items' name, which should reduce the memory consumption to some extent.
  • Removes SearchMode::Contains for Query because it's never used.
  • Merges Query::assoc_items_only and Query::exclude_import_kinds into Query::assoc_mode, because the latter is never used besides filtering associated items out.

Best reviewed one commit at a time. I made sure each commit passes full test suite. I can squash the first three commits if needed.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 30, 2023
@lowr lowr force-pushed the patch/import-map-purge-unused branch from e6d0ef3 to 5db0e1a Compare June 30, 2023 15:18
Comment on lines -449 to -452
let mut all_indexed_values = FxHashSet::default();
while let Some((_, indexed_values)) = stream.next() {
all_indexed_values.extend(indexed_values.iter().copied());
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've seen #7912, but I couldn't reproduce the duplication and I don't really think it's possible due to how fst works. Let me know if I'm missing anything.

Comment on lines 308 to 316
SearchMode::Fuzzy => {
let mut unchecked_query_chars = query_string.chars();
let mut mismatching_query_char = unchecked_query_chars.next();

for input_char in input.chars() {
match mismatching_query_char {
None => return true,
Some(matching_query_char) if matching_query_char == input_char => {
mismatching_query_char = unchecked_query_chars.next();
}
_ => (),
let mut input_chars = input.chars();
for query_char in query_string.chars() {
if input_chars.find(|&it| it == query_char).is_none() {
return false;
}
}
mismatching_query_char.is_none()
true
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This micro-optimization is based on https://rust.godbolt.org/z/jvMcT783o

@Veykril
Copy link
Member

Veykril commented Jul 3, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 3, 2023

📌 Commit 5db0e1a has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 3, 2023

⌛ Testing commit 5db0e1a with merge 691600a...

@bors
Copy link
Collaborator

bors commented Jul 3, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 691600a to master...

@bors bors merged commit 691600a into rust-lang:master Jul 3, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants