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

Rollup of 12 pull requests #102060

Closed
wants to merge 92 commits into from
Closed

Conversation

notriddle
Copy link
Contributor

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Sikkema, Isaac and others added 30 commits August 25, 2022 18:00
This is basically is ripoff of src/test/ui/simd/target-feature-mixup.rs
but for floats and without #[repr(simd)]
Co-authored-by: Laurențiu Nicola <lnicola@users.noreply.github.com>
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
Inlay hints are no longer something specifc to r-a as it has been
upstreamed into the LSP, we don't have a reason to give the config
for this feature special treatment in regards to toggling. There are
plenty of other options out there in the VSCode marketplace to create
toggle commands/hotkeys for configurations in general which I believe
we should nudge people towards instead.
The name might need some improving.

extract format_like's parser to it's own module in ide-db

reworked the parser's API to be more direct

added assist to extract expressions in format args
Added `Ident` variant to arg enum.
Previously, annotations would only appear above the name of an item (function signature, struct declaration, etc).

Now, rust-analyzer can be configured to show annotations either above the name or above the whole item (including doc comments and attributes).
Remove redundant 'resolve_obligations_as_possible' call

Hi! I was looking for a "good first issue" and saw this one: rust-lang/rust-analyzer#7542. I like searching for performance improvements, so I wanted to try to find something useful there.

There are two tests in integrated_benchmarks.rs, I looked at 'integrated_highlighting_benchmark' (not the one discussed in the issue above).

Profile from that test looks like this:
```
$ RUN_SLOW_BENCHES=1 cargo test --release --package rust-analyzer --lib -- integrated_benchmarks::integrated_highlighting_benchmark --exact --nocapture
    Finished release [optimized] target(s) in 0.06s
     Running unittests src/lib.rs (target/release/deps/rust_analyzer-a80ca6bb8f877458)

running 1 test
workspace loading: 358.45ms
initial: 9.60s
change: 13.96µs
cpu profiling is disabled, uncomment `default = [ "cpu_profiler" ]` in Cargo.toml to enable.
  273ms - highlight
      143ms - infer:wait @ per_query_memory_usage
          143ms - infer_query
                0   - crate_def_map:wait (3165 calls)
                4ms - deref_by_trait (967 calls)
               96ms - resolve_obligations_as_possible (22106 calls)
                0   - trait_solve::wait (2068 calls)
       21ms - Semantics::analyze_impl (18 calls)
        0   - SourceBinder::to_module_def (20 calls)
       36ms - classify_name (19 calls)
       19ms - classify_name_ref (308 calls)
        0   - crate_def_map:wait (461 calls)
        4ms - descend_into_macros (628 calls)
        0   - generic_params_query (4 calls)
        0   - impl_data_with_diagnostics_query (1 calls)
       45ms - infer:wait (37 calls)
        0   - resolve_obligations_as_possible (2 calls)
        0   - source_file_to_def (1 calls)
        0   - trait_solve::wait (42 calls)
after change: 275.23ms
test integrated_benchmarks::integrated_highlighting_benchmark ... ok
```
22106 calls to `resolve_obligations_as_possible` seem like the main issue there.

One thing I noticed (and fixed in this PR) is that `InferenceContext::resolve_ty_shallow` first calls `resolve_obligations_as_possible`, and then calls `InferenceTable::resolve_ty_shallow`. But `InferenceTable::resolve_ty_shallow` [inside](https://github.com/rust-lang/rust-analyzer/blob/2e9f1204ca01c3e20898d4a67c8b84899d394a88/crates/hir-ty/src/infer/unify.rs#L372) again calls `resolve_obligations_as_possible`.

`resolve_obligations_as_possible` inside has a while loop, which works until it can't find any helpful information. So calling this function for the second time does nothing, so one of the calls could be safely removed.

`InferenceContext::resolve_ty_shallow` is actually quite a hot place, and after fixing it, the total number of `resolve_obligations_as_possible` in this test is reduced to 15516 (from 22106). "After change" time also improves from ~270ms to ~240ms, which is not a very huge win, but still something measurable.

Same profile after PR:
```
$ RUN_SLOW_BENCHES=1 cargo test --release --package rust-analyzer --lib -- integrated_benchmarks::integrated_highlighting_benchmark --exact --nocapture
    Finished release [optimized] target(s) in 0.06s
     Running unittests src/lib.rs (target/release/deps/rust_analyzer-a80ca6bb8f877458)

running 1 test
workspace loading: 339.86ms
initial: 9.28s
change: 10.69µs
cpu profiling is disabled, uncomment `default = [ "cpu_profiler" ]` in Cargo.toml to enable.
  236ms - highlight
      110ms - infer:wait @ per_query_memory_usage
          110ms - infer_query
                0   - crate_def_map:wait (3165 calls)
                4ms - deref_by_trait (967 calls)
               64ms - resolve_obligations_as_possible (15516 calls)
                0   - trait_solve::wait (2068 calls)
       21ms - Semantics::analyze_impl (18 calls)
        0   - SourceBinder::to_module_def (20 calls)
       34ms - classify_name (19 calls)
       18ms - classify_name_ref (308 calls)
        0   - crate_def_map:wait (461 calls)
        3ms - descend_into_macros (628 calls)
        0   - generic_params_query (4 calls)
        0   - impl_data_with_diagnostics_query (1 calls)
       45ms - infer:wait (37 calls)
        0   - resolve_obligations_as_possible (2 calls)
        0   - source_file_to_def (1 calls)
        0   - trait_solve::wait (42 calls)
after change: 238.15ms
test integrated_benchmarks::integrated_highlighting_benchmark ... ok
```

The performance of this test could be further improved but at the cost of making code more complicated, so I wanted to check if such a change is desirable before sending another PR.

`resolve_obligations_as_possible` is actually called a lot of times even when no new information was provided. As I understand, `resolve_obligations_as_possible` could do something useful only if some variables/values were unified since the last check. We can store a boolean variable inside `InferenceTable`, which indicates if `try_unify` was called after last `resolve_obligations_as_possible`. If it wasn't called, we can safely not call `resolve_obligations_as_possible` again.

I tested this change locally, and it reduces the number of `resolve_obligations_as_possible` to several thousand (it is not shown in the profile anymore, so don't know the exact number), and the total time is reduced to ~180ms. Here is a generated profile:
```
$ RUN_SLOW_BENCHES=1 cargo test --release --package rust-analyzer --lib -- integrated_benchmarks::integrated_highlighting_benchmark --exact --nocapture
    Finished release [optimized] target(s) in 0.06s
     Running unittests src/lib.rs (target/release/deps/rust_analyzer-a80ca6bb8f877458)

running 1 test
workspace loading: 349.92ms
initial: 8.56s
change: 11.32µs
cpu profiling is disabled, uncomment `default = [ "cpu_profiler" ]` in Cargo.toml to enable.
  175ms - highlight
       21ms - Semantics::analyze_impl (18 calls)
        0   - SourceBinder::to_module_def (20 calls)
       33ms - classify_name (19 calls)
       17ms - classify_name_ref (308 calls)
        0   - crate_def_map:wait (461 calls)
        3ms - descend_into_macros (628 calls)
        0   - generic_params_query (4 calls)
        0   - impl_data_with_diagnostics_query (1 calls)
       97ms - infer:wait (38 calls)
        0   - resolve_obligations_as_possible (2 calls)
        0   - source_file_to_def (1 calls)
        0   - trait_solve::wait (42 calls)
after change: 177.04ms
test integrated_benchmarks::integrated_highlighting_benchmark ... ok
```
Let me know if adding a new bool field seems like a reasonable tradeoff, so I can send a PR.
Add config to unconditionally prefer core imports over std

Fixes rust-lang/rust-analyzer#12979
Filter imports on find-all-references

Attempt to rust-lang#13184
…eykril

fix: handle trait methods as inherent methods for trait-related types

Fixes rust-lang#10677

When resolving methods for trait object types and placeholder types that are bounded by traits, we need to count the methods of the trait and its super traits as inherent methods. This matters because these trait methods have higher priority than the other traits' methods.

Relevant code in rustc: [`assemble_inherent_candidates_from_object()`](https://github.com/rust-lang/rust/blob/0631ea5d73f4a3199c776687b12c20c50a91f0d2/compiler/rustc_typeck/src/check/method/probe.rs#L783-L792) for trait object types, [`assemble_inherent_candidates_from_param()`](https://github.com/rust-lang/rust/blob/0631ea5d73f4a3199c776687b12c20c50a91f0d2/compiler/rustc_typeck/src/check/method/probe.rs#L838-L847) for placeholder types. Notice the second arg of `push_candidate()` is `is_inherent`.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
Remove the toggleInlayHints command from VSCode

Inlay hints are no longer something specifc to r-a as it has been upstreamed into the LSP, we don't have a reason to give the config for this feature special treatment in regards to toggling. There are plenty of other options out there in the VSCode marketplace to create toggle commands/hotkeys for configurations in general which I believe we should nudge people towards instead.
GuillaumeGomez and others added 25 commits September 19, 2022 12:38
This class was originally added in 73b97c7
to support hiding and showing the item, because `main.js` went through all
`docblock` elements in the DOM and added toggles to them.

https://github.com/rust-lang/rust/blob/73b97c7e7c9cfac4dfa4804654b1db6ab687b589/src/librustdoc/html/static/main.js#L1856-L1867

The `item-decl` is no longer auto-hidden since
c96f86d removed it.

`item-decl` used to be called `type-decl`: that name was changed in
8b7a2dd.

The `docblock` class is no longer used for implementing toggles, since
rustdoc switched to using `<details>` elements.
For this rule to have an actual effect, the border-bottom width needs
specified, elsewhere, without also specifying the color. This doesn't
happen. Ever since 88b137d, every spot
where headers get a border assigned to them also assigns the color.
Since rust-lang#88679, rustdoc doesn't load crates eagerly.
Add an explicit `extern crate` item to make sure the crate is loaded and the bug reproduces.
You can verify this fix by adding `// compile-flags: -Znormalizing-docs` and running the test.
…der-no-read-perms, r=isikkema

Fix -Zmeta-stats ICE by giving `FileEncoder` file read permissions

Fixes rust-lang#101001

As far as I can tell, rust-lang#101001 is caused because the file is being created with write-only permissions here: https://github.com/rust-lang/rust/blob/master/compiler/rustc_serialize/src/opaque.rs#L196

but it is trying to be read here: https://github.com/rust-lang/rust/blob/master/compiler/rustc_metadata/src/rmeta/encoder.rs#L780

This PR attempts to fix this by creating/opening the file with the same permissions as `File::create()` with the addition of read.
…ts, r=Amanieu

Test that target feature mix up with homogeneous floats is sound

This pull-request adds a test in `src/test/abi/` that test that target feature mix up with homogeneous floats is sound.

This is basically is ripoff of [src/test/ui/simd/target-feature-mixup.rs](https://github.com/rust-lang/rust/blob/47d1cdb0bcac8e417071ce1929d261efe2399ae2/src/test/ui/simd/target-feature-mixup.rs) but for floats and without `#[repr(simd)]`.

*Extracted from rust-lang#97559 since I don't yet know what to do with that PR.*
…e, r=oli-obk

Improve error for when query is unsupported by crate

This is an improvement to the error message mentioned on rust-lang#101666.  It seems like a good idea to also add [this link to the rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/query.html), if explaining the query system in detail is a concern here, but I'm unsure if there is any restrictions on adding links to error messages.
…i-obk

MirPhase: clarify that linting is not a semantic change

r? ``@JakobDegen``
Use LLVM C-API to build atomic cmpxchg and fence

We don't need C++ wrappers because the LLVM C API can build these.
…est, r=notriddle

Add GUI test for notable traits element position

Fixes rust-lang#101891.

r? ```@notriddle```
…=compiler-errors

Simplify rpitit handling on lower_fn_decl

r? ````@compiler-errors````
some post-valtree cleanup

r? project-const-generics cc ``@b-naber``
…decl, r=GuillaumeGomez

rustdoc: remove `docblock` class from `item-decl`

This class was originally added in 73b97c7 to support hiding and showing the item, because `main.js` went through all `docblock` elements in the DOM and added toggles to them.

https://github.com/rust-lang/rust/blob/73b97c7e7c9cfac4dfa4804654b1db6ab687b589/src/librustdoc/html/static/main.js#L1856-L1867

The `item-decl` is no longer auto-hidden since c96f86d removed it.

`item-decl` used to be called `type-decl`: that name was changed in 8b7a2dd.

The `docblock` class is no longer used for implementing toggles, since rustdoc switched to using `<details>` elements.

Preview: https://notriddle.com/notriddle-rustdoc-test/docblock-item-decl/rustdoc/clean/types/enum.Type.html
… r=GuillaumeGomez

rustdoc: remove no-op CSS `h1-6 { border-bottom-color }`

For this rule to have an actual effect, the border-bottom width needs specified, elsewhere, without also specifying the color. This doesn't happen. Ever since 88b137d, every spot where headers get a border assigned to them also assigns the color.

Preview: https://notriddle.com/notriddle-rustdoc-test/border-bottom/rustc_monomorphize/collector/index.html
…ompiler-errors

Make the `normalize-overflow` rustdoc test actually do something

Since rust-lang#88679, rustdoc doesn't load crates eagerly. Add an explicit `extern crate` item to make sure the crate is loaded and the bug reproduces.
You can verify this fix by adding `// compile-flags: -Znormalize-docs` and running the test to make sure it gives an error.
…r=lnicola

⬆️ rust-analyzer

r? `@ghost`
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Sep 20, 2022
@notriddle notriddle closed this Sep 20, 2022
@notriddle notriddle deleted the rollup-hujs7kb branch September 20, 2022 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet