Skip to content

Conversation

nicolas-guichard
Copy link
Contributor

@nicolas-guichard nicolas-guichard commented Jun 30, 2025

This records the inferred type for type references in InferenceResult then uses that information in extract_type_alias to extract the inferred type.

Fixes #20108

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 30, 2025
@flodiebold
Copy link
Member

I think that could be a lot of types to store, and we'd like to avoid making the InferenceResult bigger. I wonder if we could just store the inferred types of _ types. Or alternatively we could even determine them from the inferred variable types with a bit of unification. It does make this quite a bit more complicated though, admittedly.

@nicolas-guichard
Copy link
Contributor Author

alternatively we could even determine them from the inferred variable types with a bit of unification

I don't think that's doable in the general case. In the case of LetStmt maybe, but as you say that would be a lot of work to map the selected part of the type to the correct part of the pattern. For other cases such as func::<Vec<_>>(arg0, arg1); I don't think this would work.

I'll rework this to only store the inferred _ types. Ideally this can be re-used to implement #11722 too.

@flodiebold
Copy link
Member

For other cases such as func::<Vec<_>>(arg0, arg1); I don't think this would work.

We have the recorded type of func::<Vec<_>>, so it would just be a matter of unifying that type with the lowered type where there's a variable in place of _.

Maybe before reworking it, take a look at how the memory usage changes for rust-analyzer analysis-stats with and without the patch. It could very well be not that much memory.

@ChayimFriedman2
Copy link
Contributor

@flodiebold This regressed analysis-stats only by 3mb, so I think it's acceptable.

@ChayimFriedman2
Copy link
Contributor

Unfortunately this approach isn't enough, as all the intermediate types created during lowering won't appear in the InferenceResult. For example, if there is Vec<_>, the whole Vec<_> will appear but not the _. Furthermore, solving this will be difficult as the TyLoweringContext has no knowledge of the InferenceContext (we should change that, but that needs to wait on the trait solver migration). Also, doing it properly may indeed increase memory usage.

@ShoyuVanilla ShoyuVanilla added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 10, 2025
@rustbot

This comment has been minimized.

@nicolas-guichard
Copy link
Contributor Author

This new version doesn't store the inferred type of full patterns but only the inferred type of type placeholders. This also helps towards solving #11722.

The association between placeholders' TypeRefId and the matching inference variables Ty happens in InferenceContext::make_ty by walking both the TypeRef tree and the lowered Ty. This assumes both visitations yield the same items in the same order.

SourceAnalyzer::type_of_type reuses the TypeRefIdTy mapping (and also performs the same TypeRef/Ty unification) to return the fully inferred type when possible.

I'd like to have feedback on this approach before cleaning this up. I didn't find any exiting instance of unifying a TypeRef and a Ty like this.

This still only incurs a small increase in memory usage on self analysis-stats (+4MB total).

analysis-stats output

Current master:

➜  src git:(a905e3b21b) cargo run --release --bin rust-analyzer -- analysis-stats .
Database loaded:     437.49ms, 516minstr, 96mb (metadata 170.61ms, 6747kinstr, 1244kb; build 106.70ms, 0instr, 80kb)
  item trees: 1359
  dependency lines of code: 2_307_957, item trees: 5_204
  dependency item stats: traits: 1_654, impl: 24_712, mods: 5_715, macro calls: 8_739, macro rules: 2_061
Item Tree Collection: 4.03s, 51ginstr, 836mb
  Total Statistics:
    crates: 57, mods: 1202, decls: 31308, bodies: 30250, adts: 2204, consts: 1348
  Workspace:
    traits: 114, macro_rules macros: 22, proc_macros: 1
    lines of code: 506_996, item trees: 1_359
    usages: traits: 187, impl: 4_428, mods: 1_206, macro calls: 153, macro rules: 126
  Dependencies:
    lines of code: 2_307_957, item trees: 5_204
    declarations: traits: 1_654, impl: 24_712, mods: 5_715, macro calls: 8_739, macro rules: 2_061
Item Collection:     8.60s, 59ginstr, 1076mb
Body lowering:       4.93s, 44ginstr, 780mb                                                                                                                                       
  exprs: 920132, ??ty: 873 (0%), ?ty: 990 (0%), !ty: 52                                                                                                                           
  pats: 203023, ??ty: 564 (0%), ?ty: 622 (0%), !ty: 0
  panics: 0
Inference:           108.71s, 1110ginstr, 1101mb
MIR lowering:        72.37s, 830ginstr, 835mb
Mir failed bodies: 102 (0%)
Data layouts:        89.07ms, 428minstr, 15mb
Failed data layouts: 0 (0%)
Const evaluation:    36.40ms, 248minstr, 2556kb
Failed const evals: 10 (0%)
Total:               200.04s, 2102ginstr, 2853mb

This PR:

➜  src git:(2ece970132) cargo run --release --bin rust-analyzer -- analysis-stats .
Database loaded:     412.61ms, 531minstr, 96mb (metadata 178.63ms, 13minstr, 1247kb; build 106.44ms, 5289kinstr, 80kb)
  item trees: 1359
  dependency lines of code: 2_307_957, item trees: 5_204
  dependency item stats: traits: 1_654, impl: 24_712, mods: 5_715, macro calls: 8_739, macro rules: 2_061
Item Tree Collection: 3.74s, 51ginstr, 836mb
  Total Statistics:
    crates: 57, mods: 1202, decls: 31322, bodies: 30262, adts: 2205, consts: 1348
  Workspace:
    traits: 114, macro_rules macros: 22, proc_macros: 1
    lines of code: 507_246, item trees: 1_359
    usages: traits: 187, impl: 4_429, mods: 1_206, macro calls: 153, macro rules: 126
  Dependencies:
    lines of code: 2_307_957, item trees: 5_204
    declarations: traits: 1_654, impl: 24_712, mods: 5_715, macro calls: 8_739, macro rules: 2_061
Item Collection:     8.62s, 59ginstr, 1076mb
Body lowering:       4.92s, 44ginstr, 780mb                                                                                                                                       
  exprs: 920613, ??ty: 873 (0%), ?ty: 991 (0%), !ty: 55                                                                                                                           
  pats: 203125, ??ty: 564 (0%), ?ty: 623 (0%), !ty: 0
  panics: 0
Inference:           110.12s, 1111ginstr, 1103mb
MIR lowering:        73.12s, 831ginstr, 836mb
Mir failed bodies: 105 (0%)
Data layouts:        87.68ms, 428minstr, 15mb
Failed data layouts: 0 (0%)
Const evaluation:    36.02ms, 250minstr, 2614kb
Failed const evals: 10 (0%)
Total:               201.93s, 2105ginstr, 2857mb

@nicolas-guichard nicolas-guichard force-pushed the push-pypzwzspzznu branch 3 times, most recently from 09d65ac to f208132 Compare September 1, 2025 20:03
@nicolas-guichard
Copy link
Contributor Author

The failing test is an issue with the new-solver-based type inference, see #20644.

This adds a type_of_type_placeholder arena to InferenceResult to record
which type a given type placeholder gets inferred to.
This uses the new InferenceResult::type_of_type_placeholder data to
turn type references into completely resolved types instead of just
returning the lexical type.
When met with types with placeholders, this ensures this assist
extracts the inferred type instead of the type with placeholders.

For instance, when met with this code:
```
fn main() {
    let vec: Vec<_> = vec![4];
}
```
selecting Vec<_> and extracting an alias will now yield `Vec<i32>`
instead of `Vec<_>`.
With the extra InferenceResult that maps type placeholders to their
inferred type, we can now easily display inlay hints for them.
@nicolas-guichard
Copy link
Contributor Author

Any comments on that approach that tries to unify placeholders in TypeRefs with InferenceVars in lowered Tys and stores the mapping in the InferenceResult?

(Can I change labels myself? Let's try:)
@rustbot label: -S-waiting-on-author, +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 22, 2025
@ShoyuVanilla
Copy link
Member

(Can I change labels myself? Let's try:) @rustbot label: -S-waiting-on-author, +S-waiting-on-review

Or you could try @rustbot ready for short 😄

Sorry for the delayed reviews since rust-analyzer team is currently focused on finishing trait-solver migration. I think Chayim would be the right person to review this but I'll review this if they is busy or couldn't make time for this within 2~3 days.

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.

Extract type as type alias assist does not fill infer vars
5 participants