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 #129817

Merged
merged 235 commits into from
Aug 31, 2024
Merged

Rollup of 12 pull requests #129817

merged 235 commits into from
Aug 31, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

alibektas and others added 30 commits August 1, 2024 02:38
Add an explanatory sentence and some sample code to help
readers understand why this struct exists.
minor: Add a doc comment for OpQueue

Add an explanatory sentence and some sample code to help readers understand why this struct exists.
…ebold

internal: Be more resilient to bad language item definitions in binop inference

Fixes rust-lang#16287
Fixes rust-lang#16286

There's one more in `write_fn_trait_method_resolution`, but I'm not sure if it won't cause further problems in `infer_closures`.
fix: Panic while canonicalizing erroneous projection type

Fixes rust-lang#17866

The root cause of rust-lang#17866 is quite horrifyng 😨

```rust
trait T {
    type A;
}

type Foo = <S as T>::A; // note that S isn't defined

fn main() {
    Foo {}
}
```

While inferencing alias type `Foo = <S as T>::A`;

https://github.com/rust-lang/rust-analyzer/blob/78c2bdce860dbd996a8083224d01a96660dd6a15/crates/hir-ty/src/infer.rs#L1388-L1398

the error type `S` in it is substituted by inference var in L1396 above as below;

https://github.com/rust-lang/rust-analyzer/blob/78c2bdce860dbd996a8083224d01a96660dd6a15/crates/hir-ty/src/infer/unify.rs#L866-L869

This new inference var's index is `1`, as the type inferecing procedure here previously inserted another inference var into same `InferenceTable`.

But after that, the projection type made from the above then passed to the following function;

https://github.com/rust-lang/rust-analyzer/blob/78c2bdce860dbd996a8083224d01a96660dd6a15/crates/hir-ty/src/traits.rs#L88-L96

here, a whole new `InferenceTable` is made, without any inference var and in the L94, this table calls;

https://github.com/rust-lang/rust-analyzer/blob/78c2bdce860dbd996a8083224d01a96660dd6a15/crates/hir-ty/src/infer/unify.rs#L364-L370

And while registering `AliasEq` `obligation`, this obligation contains inference var `?1` made from the previous table, but this table has only one inference var `?0` made at L365.
So, the chalk panics when we try to canonicalize that obligation to register it, because the obligation contains an inference var `?1` that the canonicalizing table doesn't have.

Currently, we are calling `InferenceTable::new()` to do some normalizing, unifying or coercing things to some targets that might contain inference var that the new table doesn't have.
I think that this is quite dangerous footgun because the inference var is just an index that does not contain the information which table does it made from, so sometimes this "foreign" index might cause panic like this case, or point at the wrong variable.

This PR mitigates such behaviour simply by inserting sufficient number of inference vars to new table to avoid such problem.
This strategy doesn't harm current r-a's intention because the inference vars that passed into new tables are just "unresolved" variables in current r-a, so this is just making sure that such "unresolved" variables exist in the new table
fix: Panic while hovering associated function with type annotation on generic param that not inherited from its container type

Fixes rust-lang#17871

We call `generic_args_sans_defaults` here;

https://github.com/rust-lang/rust-analyzer/blob/64a140527b383e3a2fe95908881624fc5374c60c/crates/hir-ty/src/display.rs#L1021-L1034

but the following substitution inside that function panic in rust-lang#17871;

https://github.com/rust-lang/rust-analyzer/blob/64a140527b383e3a2fe95908881624fc5374c60c/crates/hir-ty/src/display.rs#L1468

it's because the `Binders.binder` inside `default_parameters` has a same length with the generics of the function we are hovering on, but the generics of it is split into two, `fn_params` and `parent_params`.
Because of this, it may panic if the function has one or more default parameters and both `fn_params` and `parent_params` are non-empty, like the case in the title of this PR.

So, we must call `generic_args_sans_default` first and then split it into `fn_params` and `parent_params`
…kril

internal: Properly check the edition for edition dependent syntax kinds

This puts the current edition in a bunch of places, most of which I annoted with FIXMEs asside from the ones in ide-assists because I couldnt bother with those
Assuming it isn't cancelled. Closes rust-lang#17902.

The only place CommandHandle::join is used is when the flycheck command
finishes, so this commit changes the behavior of the method itself.
…zyCell/Lock

This doesn't get rid of the once_cell dependency, unfortunately, since we have dependencies that use it, but it's a nice to do cleanup. And when our deps will eventually get rid of once_cell we will get rid of it for free.
This speeds up short identifiers search significantly, while unlikely to have an effect on long identifiers (the analysis takes much longer than some character comparison).

Tested by finding all references to `eq()` (from `PartialEq`) in the rust-analyzer repo. Total time went down from 100s to 10s (a 10x reduction!).
internal: Replace once_cell with std's recently stabilized OnceCell/Lock and LazyCell/Lock

This doesn't get rid of the once_cell dependency, unfortunately, since we have dependencies that use it, but it's a nice to do cleanup. And when our deps will eventually get rid of once_cell we will get rid of it for free.
…s, r=Veykril

Test for word boundary in `FindUsages`

This speeds up short identifiers search significantly, while unlikely to have an effect on long identifiers (the analysis takes much longer than some character comparison).

Tested by finding all references to `eq()` (from `PartialEq`) in the rust-analyzer repo. Total time went down from 100s to 10s (a 10x reduction!).

Feel free to close this if you consider this a non-issue, as most short identifiers are local.
Allow flycheck process to exit gracefully

Assuming it isn't cancelled. Closes rust-lang#17902.

The only place CommandHandle::join() is used is when the flycheck command
finishes, so this commit changes the behavior of the method itself.

The only reason I can see for the existing behavior is if the command is somehow holding onto a build lock longer than it should, this would force it to be released. But it would be a pretty heavy-handed way to solve that issue. I'm not aware of this occurring in practice.
This PR touches a lot of parts. But the main changes are changing
`hir_expand::Name` to be raw edition-dependently and only when necessary
(unrelated to how the user originally wrote the identifier),
and changing `is_keyword()` and `is_raw_identifier()` to be edition-aware
(this was done in rust-lang#17896, but the FIXMEs were fixed here).

It is possible that I missed some cases, but most IDE parts should properly
escape (or not escape) identifiers now.

The rules of thumb are:

 - If we show the identifier to the user, its rawness should be determined
   by the edition of the edited crate. This is nice for IDE features,
   but really important for changes we insert to the source code.
 - For tests, I chose `Edition::CURRENT` (so we only have to (maybe) update
   tests when an edition becomes stable, to avoid churn).
 - For debugging tools (helper methods and logs), I used `Edition::LATEST`.
…-keyword, r=Veykril

fix: Properly account for editions in names

This PR touches a lot of parts. But the main changes are changing `hir_expand::Name` to be raw edition-dependently and only when necessary (unrelated to how the user originally wrote the identifier), and changing `is_keyword()` and `is_raw_identifier()` to be edition-aware (this was done in rust-lang#17896, but the FIXMEs were fixed here).

It is possible that I missed some cases, but most IDE parts should properly escape (or not escape) identifiers now.

The rules of thumb are:

 - If we show the identifier to the user, its rawness should be determined by the edition of the edited crate. This is nice for IDE features, but really important for changes we insert to the source code.
 - For tests, I chose `Edition::CURRENT` (so we only have to (maybe) update tests when an edition becomes stable, to avoid churn).
 - For debugging tools (helper methods and logs), I used `Edition::LATEST`.

Reviewing notes:

This is a really big PR but most of it is mechanical translation. I changed `Name` displayers to require an edition, and followed the compiler errors. Most methods just propagate the edition requirement. The interesting cases are mostly in `ide-assists`, as sometimes the correct crate to fetch the edition from requires awareness (there may be two). `ide-completions` and `ide-diagnostics` were solved pretty easily by introducing an edition field to their context. `ide` contains many features, for most of them it was propagated to the top level function and there the edition was fetched based on the file.

I also fixed all FIXMEs from rust-lang#17896. Some required introducing an edition parameter (usually not for many methods after the changes to `Name`), some were changed to a new method `is_any_identifier()` because they really want any possible keyword.

Fixes rust-lang#17895.
Fixes rust-lang#17774.
…r=davidbarsky

Add scip/lsif flag to exclude vendored libaries

rust-lang#17809 changed StaticIndex to include vendored libraries. This PR adds a flag to disable that behavior.

At work, our monorepo has too many rust targets to index all at once, so we split them up into several shards. Since all of our libraries are vendored, if rust-analyzer includes them, sharding no longer has much benefit, because every shard will have to index the entire transitive dependency graphs of all of its targets. We get around the issue presented in rust-lang#17809 because some other shard will index the libraries directly.
…, r=lnicola

Remove rust-analyzer.workspace.discoverProjectRunner

The functionality for this vscode config option was removed in rust-lang#17395, so it doesn't do anything anymore.
Pin `rowan` to `0.15.15`

To prevent rust-lang#17914, I think that it would be safer pinning this before we fix it correctly
@rustbot rustbot added T-libs Relevant to the library 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels Aug 31, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=13

@bors
Copy link
Contributor

bors commented Aug 31, 2024

📌 Commit 830b1de has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 31, 2024
@bors
Copy link
Contributor

bors commented Aug 31, 2024

⌛ Testing commit 830b1de with merge 3f329d5...

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2024
…iaskrgr

Rollup of 12 pull requests

Successful merges:

 - rust-lang#129659 (const fn stability checking: also check declared language features)
 - rust-lang#129711 (Expand NLL MIR dumps)
 - rust-lang#129730 (f32 docs: define 'arithmetic' operations)
 - rust-lang#129733 (Subtree update of `rust-analyzer`)
 - rust-lang#129749 (llvm-wrapper: adapt for LLVM API changes)
 - rust-lang#129757 (Add a test for trait solver overflow in MIR inliner cycle detection)
 - rust-lang#129760 (Make the "detect-old-time" UI test more representative)
 - rust-lang#129767 (Remove `#[macro_use] extern crate tracing`, round 4)
 - rust-lang#129774 (Remove `#[macro_use] extern crate tracing` from rustdoc and rustfmt)
 - rust-lang#129785 (Miri subtree update)
 - rust-lang#129791 (mark joboet as on vacation)
 - rust-lang#129812 (interpret, codegen: tweak some comments and checks regarding Box with custom allocator)

Failed merges:

 - rust-lang#129777 (Add `unreachable_pub`, round 4)

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] miri test:false 4.448
error: failed to remove file `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe`

Caused by:
  Access is denied. (os error 5)
Command has failed. Rerun with -v to see more details.
  local time: Sat, Aug 31, 2024  3:34:14 PM
  network time: Sat, 31 Aug 2024 15:34:15 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@bors
Copy link
Contributor

bors commented Aug 31, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 31, 2024
@matthiaskrgr
Copy link
Member Author

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 31, 2024
@bors
Copy link
Contributor

bors commented Aug 31, 2024

⌛ Testing commit 830b1de with merge d571ae8...

@bors
Copy link
Contributor

bors commented Aug 31, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing d571ae8 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 31, 2024
@bors bors merged commit d571ae8 into rust-lang:master Aug 31, 2024
7 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 31, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#129659 const fn stability checking: also check declared language f… 7b783a0cd6e5a8c5069122db86545a230d74c92c (link)
#129711 Expand NLL MIR dumps fe4f400f1e00fea8f078eb2d0692510f63f8ce9b (link)
#129730 f32 docs: define 'arithmetic' operations 83675a3dca5431332d519d808c8cfaa7461235f8 (link)
#129733 Subtree update of rust-analyzer 8fcd87d4502ccee54c5b73290ae6334901de14da (link)
#129749 llvm-wrapper: adapt for LLVM API changes 00a3a8b8074bb39d5bb4fbfa89da7ce896b4149d (link)
#129757 Add a test for trait solver overflow in MIR inliner cycle d… 0157c371f8ed14cd4d79570073e3bfa1883aa6cc (link)
#129760 Make the "detect-old-time" UI test more representative 3028f87bc3ac8b57ac5badce5139bab5313296ab (link)
#129767 Remove #[macro_use] extern crate tracing, round 4 4f0a0da05ed1dfed1f95ccce854de8bcf013a64c (link)
#129774 Remove #[macro_use] extern crate tracing from rustdoc and… d3569ae235eb03c98350985fe116870a5553a2de (link)
#129785 Miri subtree update 5923b1e4d8cacb5287504e6093206cfcfaf47107 (link)
#129791 mark joboet as on vacation 2846e20654e8c5ae1506cdc94ce94de8d9ea1db9 (link)
#129812 interpret, codegen: tweak some comments and checks regardin… cbec43d999f5835cd45aed375f840db9f8c29e84 (link)

previous master: 9649706ead

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d571ae8): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.6% [2.6%, 2.6%] 1
Regressions ❌
(secondary)
0.5% [0.3%, 0.6%] 9
Improvements ✅
(primary)
-0.5% [-0.5%, -0.5%] 1
Improvements ✅
(secondary)
-0.7% [-2.0%, -0.4%] 8
All ❌✅ (primary) 1.1% [-0.5%, 2.6%] 2

Max RSS (memory usage)

Results (primary -0.5%, secondary -0.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.0% [0.4%, 2.3%] 11
Regressions ❌
(secondary)
1.0% [0.5%, 3.2%] 20
Improvements ✅
(primary)
-0.9% [-2.5%, -0.4%] 46
Improvements ✅
(secondary)
-0.7% [-3.4%, -0.4%] 46
All ❌✅ (primary) -0.5% [-2.5%, 2.3%] 57

Cycles

Results (primary -0.3%, secondary 0.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.7% [0.4%, 2.4%] 18
Regressions ❌
(secondary)
1.1% [0.4%, 4.5%] 56
Improvements ✅
(primary)
-1.9% [-4.7%, -0.4%] 11
Improvements ✅
(secondary)
-1.1% [-4.1%, -0.4%] 45
All ❌✅ (primary) -0.3% [-4.7%, 2.4%] 29

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 788.814s -> 789.576s (0.10%)
Artifact size: 338.74 MiB -> 338.70 MiB (-0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Aug 31, 2024
@matthiaskrgr matthiaskrgr deleted the rollup-ll2ld5m branch September 1, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.