Skip to content

feat: Add "make tuple" tactic to term search #16687

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

Merged
merged 3 commits into from
Feb 27, 2024
Merged

Conversation

kilpkonn
Copy link
Contributor

@kilpkonn kilpkonn commented Feb 26, 2024

Follow up to #16092

Now term search also supports tuples.

let a: i32 = 1;
let b: f64 = 0.0;
let c: (i32, (f64, i32)) = todo!(); // Finds (a, (b, a))

In addition to new tactic that handles tuples I changed how the generics are handled.
Previously it tried all possible options from types we had in scope but now it only tries useful ones that help us directly towards the goal or at least towards calling some other function.
This changes O(2^n) to O(n^2) where n is amount of rounds which in practice allows using types that take generics for multiple rounds (previously limited to 1). Average case that also used to be exponential is now roughly linear.
This means that deeply nested generics also work.

// Finds all valid combos, including `Some(Some(Some(...)))`
let a: Option<Option<Option<bool>>> = todo!();

Note that although the complexity is smaller allowing more types with generics the search overall slows down considerably. I hope it's fine tho as the autocomplete is disabled by default and for code actions it's not super slow. Might have to tweak the depth hyper parameter tho

This resulted in a huge increase of results found (benchmarks on ripgrep crate):
Before

Tail Expr syntactic hits: 149/1692 (8%)
Tail Exprs found: 749/1692 (44%)
Term search avg time: 18ms

After

Tail Expr syntactic hits: 291/1692 (17%)
Tail Exprs found: 1253/1692 (74%)
Term search avg time: 139ms

Most changes are local to term search except some tuple related stuff on hir::Type.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 26, 2024
"0277" if generated.contains(&todo) => continue, // See https://github.com/rust-lang/rust/issues/69882
"0282" | "0283" => continue, // Byproduct of testing method
"0277" | "0308" if generated.contains(&todo) => continue, // See https://github.com/rust-lang/rust/issues/69882
// FIXME: In some rare cases `AssocItem::container_or_implemented_trait` returns `None` for trait methods.
Copy link
Member

Choose a reason for hiding this comment

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

That does not sound good 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah 😛
It'happened once for FromStr::from_str in ripgrep crate. Got no idea how that case was special tho

@Veykril
Copy link
Member

Veykril commented Feb 27, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Feb 27, 2024

📌 Commit a2bf15e has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Feb 27, 2024

⌛ Testing commit a2bf15e with merge 6b250a2...

@bors
Copy link
Contributor

bors commented Feb 27, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 6b250a2 to master...

@bors bors merged commit 6b250a2 into rust-lang:master Feb 27, 2024
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.

4 participants