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 6 pull requests #135615

Merged
merged 18 commits into from
Jan 17, 2025
Merged

Rollup of 6 pull requests #135615

merged 18 commits into from
Jan 17, 2025

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

lqd and others added 18 commits January 12, 2025 07:29
in NLLs some locals are marked live at all points if one of their
regions escapes the function but that doesn't work in a flow-sensitive
setting like polonius
we're in in the endgame now

set up the location-sensitive analysis end to end:
- stop recording inflowing loans and loan liveness in liveness
- replace location-insensitive liveness data with live loans computed by
  reachability
- remove equivalence between polonius scopes and NLL scopes, and only
  run one scope computation
it's a bit mind-bending
this addresses review comments while:
- keeping the symmetry between the NLL and Polonius out of scope
  precomputers
- keeping the unstable `calculate_borrows_out_of_scope_at_location`
  function to avoid churn for consumers
```
error[E0308]: `if` and `else` have incompatible types
  --> $DIR/if-else-chain-missing-else.rs:12:12
   |
LL |        let x = if let Ok(x) = res {
   |  ______________-
LL | |          x
   | |          - expected because of this
LL | |      } else if let Err(e) = res {
   | | ____________^
LL | ||         return Err(e);
LL | ||     };
   | ||     ^
   | ||_____|
   |  |_____`if` and `else` have incompatible types
   |        expected `i32`, found `()`
   |
   = note: `if` expressions without `else` evaluate to `()`
   = note: consider adding an `else` block that evaluates to the expected type
```

We probably want a longer explanation and fewer spans on this case.

Partially address rust-lang#133316.
constants and statics are nullary functions, and struct fields are unary functions.

functions (along with methods and trait methods) are prioritized over other
items, like fields and constants.
…-func, r=notriddle

Treat other items as functions for the purpose of type-based search

specifically, constants and statics are nullary functions, and struct fields are unary functions.

fixes rust-lang#130204

r? ``@notriddle``
…kh726

Location-sensitive polonius prototype: endgame

This PR sets up the naive location-sensitive analysis end-to-end, and replaces the location-insensitive analysis. It's roughly all the in-progress work I wanted to land for the prototype, modulo cleanups I still want to do after the holidays, or the polonius debugger, and so on.

Here, we traverse the localized constraint graph, have to deal with kills and time-traveling (👌), and record that as loan liveness for the existing scope and active loans computations.

Then the near future looks like this, especially if the 2025h1 project goal is accepted:
- gradually bringing it up to completion
- analyzing and fixing the few remaining test failures
- going over the *numerous* fixmes in this prototype (one of which is similar to a hang on one test's millions and millions of constraints)
- trying to see how to lower the impact of the lack of NLL liveness optimization on diagnostics, and their categorization of local variables and temporaries (the vast majority of blessed expectations differences), as well as the couple ICEs trying to find an NLL constraint to blame for errors.
- dealing with the theoretical weakness around kills, conflating reachability for the two TCS, etc that is described ad nauseam in the code.
- switching the compare mode to the in-tree implementation, and blessing the diagnostics
- apart from the hang, it's not catastrophically slower on our test suite, so then we can try to enable it on CI
- checking crater, maybe trying to make it faster :3, etc.

I've tried to gradually introduce this PR's work over 4 commits, because it's kind of subtle/annoying, and Niko/I are not completely convinced yet. That one comment explaining the situation is maybe 30% of the PR 😓. Who knew that spacetime reachability and time-traveling could be mind bending.

I kinda found this late and the impact on this part of the computation was a bit unexpected to us. A bit more care/thought will be needed here. I've described my plan in the comments though. In any case, I believe we have the current implementation is a conservative approximation that shouldn't result in unsoundness but false positives at worst. So it feels fine for now.

r? ``@jackh726``

---

Fixes rust-lang#127628 -- which was a assertion triggered for a difference in loan computation between NLLs and the location-insensitive analysis. That doesn't exist anymore so I've removed this crash test.
Detect if-else chains with a missing final else in type errors

```
error[E0308]: `if` and `else` have incompatible types
  --> $DIR/if-else-chain-missing-else.rs:12:12
   |
LL |        let x = if let Ok(x) = res {
   |  ______________-
LL | |          x
   | |          - expected because of this
LL | |      } else if let Err(e) = res {
   | | ____________^
LL | ||         return Err(e);
LL | ||     };
   | ||     ^
   | ||_____|
   |  |_____`if` and `else` have incompatible types
   |        expected `i32`, found `()`
   |
   = note: `if` expressions without `else` evaluate to `()`
   = note: consider adding an `else` block that evaluates to the expected type
```

We probably want a longer explanation and fewer spans on this case.

Partially address rust-lang#133316.
…, r=notriddle

fix error for when results in a rustdoc-js test are in the wrong order

see rust-lang#131806 (comment)
…=jieyouxu

Fix suggestion to convert dereference of raw pointer to ref

Fix rust-lang#135580
Expand docs for `E0207` with additional example

Add an example to E0207 docs showing how to tie the lifetime of the self type to an associated type in an impl when the trait *doesn't* have a lifetime to begin with.

CC rust-lang#135589.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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 Jan 17, 2025
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=6

@bors
Copy link
Contributor

bors commented Jan 17, 2025

📌 Commit 1360e76 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 Jan 17, 2025
@bors
Copy link
Contributor

bors commented Jan 17, 2025

⌛ Testing commit 1360e76 with merge 73c0ae6...

@bors
Copy link
Contributor

bors commented Jan 17, 2025

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 17, 2025
@bors bors merged commit 73c0ae6 into rust-lang:master Jan 17, 2025
7 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 17, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#131806 Treat other items as functions for the purpose of type-base… 285c72d5126f014e3d4741f7932be0582e2fa62a (link)
#134980 Location-sensitive polonius prototype: endgame ceb58f782d0c7c8bb8011f6e36ca4b3c97cb77b9 (link)
#135558 Detect if-else chains with a missing final else in type err… 8562ed354ee24a26e1d774e4fb05b3c5b34dcd7f (link)
#135594 fix error for when results in a rustdoc-js test are in the … 3bc2f411000071a492c4c442fe6173e9d9f37755 (link)
#135601 Fix suggestion to convert dereference of raw pointer to ref 67cea77d8426f10695aa0704acf6074b4dcfee67 (link)
#135604 Expand docs for E0207 with additional example 528d759beb16b72c097aa76b8a5048a4348419bd (link)

previous master: 0c2c096e1a

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 (73c0ae6): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
0.3% [0.3%, 0.3%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% [0.3%, 0.3%] 1

Max RSS (memory usage)

Results (primary 1.8%, secondary 1.7%)

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.8% [1.8%, 1.8%] 1
Regressions ❌
(secondary)
3.6% [1.7%, 5.5%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) 1.8% [1.8%, 1.8%] 1

Cycles

Results (primary -1.7%)

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
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.7% [-2.4%, -1.3%] 4
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.7% [-2.4%, -1.3%] 4

Binary size

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

Bootstrap: 764.62s -> 764.673s (0.01%)
Artifact size: 326.02 MiB -> 326.10 MiB (0.03%)

@matthiaskrgr matthiaskrgr deleted the rollup-ra7vftt branch January 25, 2025 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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-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.

8 participants