Fix Assist "replace named generic type with impl trait"
This is a follow-up PR to fix the assist "replace named generic type with impl trait" described in #14626 to filter invalid param types. It integrates the feedback given in PR #14816 .
The change updates the logic to determine when a function parameter is safe to replace a type param with its trait implementation. Some parameter definitions are invalid & should not be replaced by their traits, therefore skipping the assist completely.
First, all usages of the generic type under the cursor are determined. These usage references are checked to see if they occur outside the function parameter list. If an outside reference is found, e.g. in body, return type or where clause, the assist is skipped. All remaining usages need to appear only in the function param list. For each usage the param type is further inspected to see if it's valid. The logic to determine if a function parameter is valid, follows a heuristic and may not cover all possible parameter definitions.
With this change the following param types (as given in [this comment](https://github.com/rust-lang/rust-analyzer/pull/14816#discussion_r1206834603)) are not replaced & therefore skip the assist.
```rust
fn foo<P: Trait>(
_: <P as Trait>::Assoc, // within path type qualifier
_: <() as OtherTrait<P>>::Assoc, // same as above
_: P::Assoc, // associated type shorthand
_: impl OtherTrait<P> // generic arg in impl trait (note that associated type bindings are fine)
_: &dyn Fn(P) // param type and/or return type for Fn* traits
) {}
```
The change updates the logic to determine if a function parameter is
valid for replacing the type param with the trait implementation.
First all usages are determined, to check if they are used outside the function
parameter list. If an outside reference is found, e.g. in body, return type or
where clause, the assist is skipped. All remaining usages only appear in the
function param list. For each usage the param type is checked to see if
it's valid.
**Please note** the logic currently follows a heuristic and may not cover
all existing parameter declarations.
* determine valid usage references by checking ancestors (on AST level)
* split test into separate ones
fix: Fix nav target calculation discarding file ids from differing macro upmapping
Fixes https://github.com/rust-lang/rust-analyzer/issues/14792
Turns out there was the assumption that upmapping from a macro will always end in the same root file, which is no longer the case thanks to `include!`
Add signature help for tuple patterns and expressions
~~These are somewhat wonky since their signature changes as you type depending on context but they help out nevertheless.~~ should be less wonky now with added parser and lowering recoveries
fix: Don't duplicate sysroot crates in rustc workspace
Since we handle `library` as the sysroot source directly in the rustc workspace, we now duplicate the crates there, once as sysroot and once as just plain workspace crate. This causes a variety of issues for `vec!` macros and similar that emit `$crate` tokens across crates.
Prioritize threads affected by user typing
To this end I’ve introduced a new custom thread pool type which can spawn threads using each QoS class. This way we can run latency-sensitive requests under one QoS class and everything else under another QoS class. The implementation is very similar to that of the `threadpool` crate (which is currently used by rust-analyzer) but with unused functionality stripped out.
I’ll have to rebase on master once #14859 is merged but I think everything else is alright :D
Fix edits for `convert_named_struct_to_tuple_struct`
Two fixes:
- When replacing syntax nodes, macro files weren't taken into account. Edits were simply made for `node.syntax().text_range()`, which would be wrong range when `node` is inside a macro file.
- We do ancestor node traversal for every struct name reference to find record expressions/patterns to edit, but we didn't verify that expressions/patterns do actually refer to the struct we're operating on.
Best reviewed one commit at a time.
Fixes#13780Fixes#14927
Previously we didn't verify that record expressions/patterns that were
found did actually point to the struct we're operating on. Moreover,
when that record expressions/patterns had missing child nodes, we would
continue traversing their ancestor nodes.
This code replaces the thread pool implementation we were using
previously (from the `threadpool` crate). By making the thread pool
aware of QoS, each job spawned on the thread pool can have a different
QoS class.
This commit also replaces every QoS class used previously with Default
as a temporary measure so that each usage can be chosen deliberately.
Specify thread types using Quality of Service API
<details>
<summary>Some background (in case you haven’t heard of QoS before)</summary>
Heterogenous multi-core CPUs are increasingly found in laptops and desktops (e.g. Alder Lake, Snapdragon 8cx Gen 3, M1). To maximize efficiency on this kind of hardware, it is important to provide the operating system with more information so threads can be scheduled on different core types appropriately.
The approach that XNU (the kernel of macOS, iOS, etc) and Windows have taken is to provide a high-level semantic API – quality of service, or QoS – which informs the OS of the program’s intent. For instance, you might specify that a thread is running a render loop for a game. This makes the OS provide this thread with as large a share of the system’s resources as possible. Specifying a thread is running an unimportant background task, on the other hand, is cause for it to be scheduled exclusively on high-efficiency cores instead of high-performance cores.
QoS APIs allows for easy configuration of many different parameters at once; for instance, setting QoS on XNU affects scheduling, timer latency, I/O priorities, and of course what core type the thread in question should run on. I don’t know any details on how QoS works on Windows, but I would guess it’s similar.
Hypothetically, taking advantage of these APIs would improve power consumption, thermals, battery life if applicable, etc.
</details>
# Relevance to rust-analyzer
From what I can tell the philosophy behind both the XNU and Windows QoS APIs is that _user interfaces should never stutter under any circumstances._ You can see this in the array of QoS classes which are available: the highest QoS class in both APIs is one intended explicitly for UI render loops.
Imagine rust-analyzer is performing CPU-intensive background work – maybe you just invoked Find Usages on `usize` or opened a large project – in this scenario the editor’s render loop should absolutely get higher priority than rust-analyzer, no matter what. You could view it in terms of “realtime-ness”: flight control software is hard realtime, audio software is soft realtime, GUIs are softer realtime, and rust-analyzer is not realtime at all. Of course, maximizing responsiveness is important, but respecting the rest of the system is more important.
# Implementation
I’ve tried my best to unify thread creation in `stdx`, where the new API I’ve introduced _requires_ specifying a QoS class. Different points along the performance/efficiency curve can make a great difference; the M1’s e-cores use around three times less power than the p-cores, so putting in this effort is worthwhile IMO.
It’s worth mentioning that Linux does not [yet](https://youtu.be/RfgPWpTwTQo) have a QoS API. Maybe translating QoS into regular thread priorities would be acceptable? From what I can tell the only scheduling-related code in rust-analyzer is Windows-specific, so ignoring QoS entirely on Linux shouldn’t cause any new issues. Also, I haven’t implemented support for the Windows QoS APIs because I don’t have a Windows machine to test on, and because I’m completely unfamiliar with Windows APIs :)
I noticed that rust-analyzer handles some requests on the main thread (using `.on_sync()`) and others on a threadpool (using `.on()`). I think it would make sense to run the main thread at the User Initiated QoS and the threadpool at Utility, but only if all requests that are caused by typing use `.on_sync()` and all that don’t use `.on()`. I don’t understand how the `.on_sync()`/`.on()` split that’s currently present was chosen, so I’ve let this code be for the moment. Let me know if changing this to what I proposed makes any sense.
To avoid having to change everything back in case I’ve misunderstood something, I’ve left all threads at the Utility QoS for now. Of course, this isn’t what I hope the code will look like in the end, but I figured I have to start somewhere :P
# References
<ul>
<li><a href="https://developer.apple.com/library/archive/documentation/Performance/Conceptual/power_efficiency_guidelines_osx/PrioritizeWorkAtTheTaskLevel.html">Apple documentation related to QoS</a></li>
<li><a href="67e155c940/include/pthread/qos.h">pthread API for setting QoS on XNU</a></li>
<li><a href="https://learn.microsoft.com/en-us/windows/win32/procthread/quality-of-service">Windows’s QoS classes</a></li>
<li>
<details>
<summary>Full documentation of XNU QoS classes. This documentation is only available as a huge not-very-readable comment in a header file, so I’ve reformatted it and put it here for reference.</summary>
<ul>
<li><p><strong><code>QOS_CLASS_USER_INTERACTIVE</code>: A QOS class which indicates work performed by this thread is interactive with the user.</strong></p><p>Such work is requested to run at high priority relative to other work on the system. Specifying this QOS class is a request to run with nearly all available system CPU and I/O bandwidth even under contention. This is not an energy-efficient QOS class to use for large tasks. The use of this QOS class should be limited to critical interaction with the user such as handling events on the main event loop, view drawing, animation, etc.</p></li>
<li><p><strong><code>QOS_CLASS_USER_INITIATED</code>: A QOS class which indicates work performed by this thread was initiated by the user and that the user is likely waiting for the results.</strong></p><p>Such work is requested to run at a priority below critical user-interactive work, but relatively higher than other work on the system. This is not an energy-efficient QOS class to use for large tasks. Its use should be limited to operations of short enough duration that the user is unlikely to switch tasks while waiting for the results. Typical user-initiated work will have progress indicated by the display of placeholder content or modal user interface.</p></li>
<li><p><strong><code>QOS_CLASS_DEFAULT</code>: A default QOS class used by the system in cases where more specific QOS class information is not available.</strong></p><p>Such work is requested to run at a priority below critical user-interactive and user-initiated work, but relatively higher than utility and background tasks. Threads created by <code>pthread_create()</code> without an attribute specifying a QOS class will default to <code>QOS_CLASS_DEFAULT</code>. This QOS class value is not intended to be used as a work classification, it should only be set when propagating or restoring QOS class values provided by the system.</p></li>
<li><p><strong><code>QOS_CLASS_UTILITY</code>: A QOS class which indicates work performed by this thread may or may not be initiated by the user and that the user is unlikely to be immediately waiting for the results.</strong></p><p>Such work is requested to run at a priority below critical user-interactive and user-initiated work, but relatively higher than low-level system maintenance tasks. The use of this QOS class indicates the work should be run in an energy and thermally-efficient manner. The progress of utility work may or may not be indicated to the user, but the effect of such work is user-visible.</p></li>
<li><p><strong><code>QOS_CLASS_BACKGROUND</code>: A QOS class which indicates work performed by this thread was not initiated by the user and that the user may be unaware of the results.</strong></p><p>Such work is requested to run at a priority below other work. The use of this QOS class indicates the work should be run in the most energy and thermally-efficient manner.</p></li>
<li><p><strong><code>QOS_CLASS_UNSPECIFIED</code>: A QOS class value which indicates the absence or removal of QOS class information.</strong></p><p>As an API return value, may indicate that threads or pthread attributes were configured with legacy API incompatible or in conflict with the QOS class system.</p></li>
</ul>
</details>
</li>
</ul>
feat: Assist to replace generic with impl trait
This adds a new assist named "Replace named generic with impl". It is the inverse operation to the existing "Replace impl trait with generic" assist.
It allows to refactor the following statement:
```rust
// 👇 cursor
fn new<T$0: ToString>(input: T) -> Self {}
```
to be transformed into:
```rust
fn new(input: impl ToString) -> Self {}
```
* adds new helper function `impl_trait_type` to create AST node
* add method to remove an existing generic param type from param list
Closes#14626
This removes an existing generic param from the `GenericParamList`. It
also considers to remove the extra colon & whitespace to the previous
sibling.
* change order to get all param types first and mark them as mutable
before the first edit happens
* add helper function to remove a generic parameter
* fix test output
This adds a new assist named "replace named generic with impl" to move
the generic param type from the generic param list into the function
signature.
```rust
fn new<T: ToString>(input: T) -> Self {}
```
becomes
```rust
fn new(input: impl ToString) -> Self {}
```
The first step is to determine if the assist can be applied, there has
to be a match between generic trait param & function paramter types.
* replace function parameter type(s) with impl
* add new `impl_trait_type` function to generate the new trait bounds with `impl` keyword for use in the
function signature
fix: assists no longer break indentation
Fixes https://github.com/rust-lang/rust-analyzer/issues/14674
These are _ad hoc_ patches for a number of assists that can produce incorrectly indented code, namely:
- generate_derive
- add_missing_impl_members
- add_missing_default_members
Some general solution is required in future, as the same problem arises in many other assists, e.g.
- replace_derive_with...
- generate_default_from_enum...
- generate_default_from_new
- generate_delegate_methods
(the list is incomplete)
Fix: a TODO and some clippy fixes
- fix(todo): implement IntoIterator for ArenaMap<IDX, V>
- chore: remove unused method
- fix: remove useless `return`s
- fix: various clippy lints
- fix: simplify boolean test to a single negation
fix: introduce new type var when expectation for ref pat is not ref
Fixes#14840
When we infer the type of ref patterns, its expected type may not be reference type: 1) expected type is an unresolved inference variable, or 2) expected type is erroneously other kind of type. In either case, we should produce a reference type with a new type variable rather than an error type so that we can continue inferring the inner patterns without further errors because of the (possible) type mismatch of this pattern.
fix: consider all tokens in macro expr when analyzing locals
Fixes#14687
2 fixes for `extract_function` assist (related closely enough that I squashed into one commit):
- Locals in macro expressions have been analyzed only when they are in the top-level token tree the macro call wraps. We should consider all descendant tokens.
- `self` in macro expressions haven't been analyzed.
