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 7 pull requests #133120

Merged
merged 18 commits into from
Nov 17, 2024
Merged

Rollup of 7 pull requests #133120

merged 18 commits into from
Nov 17, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

tgross35 and others added 18 commits October 29, 2024 01:44
The API is already stable since [1], but const stability was blocked on
`const_mut_refs`. Since that was recently stabilized, const stabilize
the following:

    // core::atomic

    impl AtomicBool { pub const unsafe fn from_ptr<'a>(ptr: *mut bool) -> &'a AtomicBool; }

    impl<T> AtomicPtr<T> { pub const unsafe fn from_ptr<'a>(ptr: *mut *mut T) -> &'a AtomicPtr<T>; }

    impl AtomicU8    { pub const unsafe fn from_ptr<'a>(ptr: *mut u8)    -> &'a AtomicU8;    }
    impl AtomicU16   { pub const unsafe fn from_ptr<'a>(ptr: *mut u16)   -> &'a AtomicU16;   }
    impl AtomicU32   { pub const unsafe fn from_ptr<'a>(ptr: *mut u32)   -> &'a AtomicU32;   }
    impl AtomicU64   { pub const unsafe fn from_ptr<'a>(ptr: *mut u64)   -> &'a AtomicU64;   }
    impl AtomicUsize { pub const unsafe fn from_ptr<'a>(ptr: *mut usize) -> &'a AtomicUsize; }

    impl AtomicI8    { pub const unsafe fn from_ptr<'a>(ptr: *mut i8)    -> &'a AtomicI8;    }
    impl AtomicI16   { pub const unsafe fn from_ptr<'a>(ptr: *mut i16)   -> &'a AtomicI16;   }
    impl AtomicI32   { pub const unsafe fn from_ptr<'a>(ptr: *mut i32)   -> &'a AtomicI32;   }
    impl AtomicI64   { pub const unsafe fn from_ptr<'a>(ptr: *mut i64)   -> &'a AtomicI64;   }
    impl AtomicIsize { pub const unsafe fn from_ptr<'a>(ptr: *mut isize) -> &'a AtomicIsize; }

Closes: <rust-lang#108652>
[1]: <rust-lang#115719>
The results of most analyses end up in a `Results<'tcx, A>`, where `A`
is the analysis. It's then possible to traverse the results via a
`ResultsVisitor`, which relies on the `ResultsVisitable` trait. (That
trait ends up using the same `apply_*` methods that were used when
computing the analysis, albeit indirectly.)

This pattern of "compute analysis results, then visit them" is common.
But there is one exception. For borrow checking we compute three
separate analyses (`Borrows`, `MaybeUninitializedPlaces`, and
`EverInitializedPlaces`), combine them into a single `BorrowckResults`,
and then do a single visit of that `BorrowckResults` with
`MirBorrowckResults`. `BorrowckResults` is just different enough from
`Results` that it requires the existence of `ResultsVisitable`, which
abstracts over the traversal differences between `Results` and
`BorrowckResults`.

This commit changes things by introducing `Borrowck` and bundling the
three borrowck analysis results into a standard `Results<Borrowck>`
instead of the exceptional `BorrowckResults`. Once that's done, the
results can be visited like any other analysis results.
`BorrowckResults` is removed, as is `impl ResultsVisitable for
BorrowckResults`. (It's instructive to see how similar the added `impl
Analysis for Borrowck` is to the removed `impl ResultsVisitable for
BorrowckResults`. They're both doing exactly the same things.)

Overall this increases the number of lines of code and might not seem
like a win. But it enables the removal of `ResultsVisitable` in the next
commit, which results in many simplifications.
Now that `Results` is the only impl of `ResultsVisitable`, the trait can
be removed. This simplifies things by removining unnecessary layers of
indirection and abstraction.

- `ResultsVisitor` is simpler.
  - Its type parameter changes from `R` (an analysis result) to the
    simpler `A` (an analysis).
  - It no longer needs the `Domain` associated type, because it can use
    `A::Domain`.
  - Occurrences of `R` become `Results<'tcx, A>`, because there is now
    only one kind of analysis results.

- `save_as_intervals` also changes type parameter from `R` to `A`.

- The `results.reconstruct_*` method calls are replaced with
  `results.analysis.apply_*` method calls, which are equivalent.

- `Direction::visit_results_in_block` is simpler, with a single generic
  param (`A`) instead of two (`D` and `R`/`F`, with a bound connecting
  them). Likewise for `visit_results`.

- The `ResultsVisitor` impls for `MirBorrowCtxt` and
  `StorageConflictVisitor` are now specific about the type of the
  analysis results they work with. They both used to have a type param
  `R` but they weren't genuinely generic. In both cases there was only a
  single results type that made sense to instantiate them with.
fixes rust-lang#129707

this can be used to show all items in a module,
or all associated items for a type.
currently sufferes slightly due to case insensitivity,
so `Option::` will also show items in the `option::` module.

it disables the checking of the last path element,
otherwise only items with short names will be shown
…om_ptr, r=RalfJung

Stabilize `const_atomic_from_ptr`

The API is already stable since rust-lang#115719, but const stability was blocked on `const_mut_refs`. Since that was recently stabilized, const stabilize the following:

```rust
// core::atomic

impl AtomicBool { pub const unsafe fn from_ptr<'a>(ptr: *mut bool) -> &'a AtomicBool; }

impl<T> AtomicPtr<T> { pub const unsafe fn from_ptr<'a>(ptr: *mut *mut T) -> &'a AtomicPtr<T>; }

impl AtomicU8    { pub const unsafe fn from_ptr<'a>(ptr: *mut u8)    -> &'a AtomicU8;    }
impl AtomicU16   { pub const unsafe fn from_ptr<'a>(ptr: *mut u16)   -> &'a AtomicU16;   }
impl AtomicU32   { pub const unsafe fn from_ptr<'a>(ptr: *mut u32)   -> &'a AtomicU32;   }
impl AtomicU64   { pub const unsafe fn from_ptr<'a>(ptr: *mut u64)   -> &'a AtomicU64;   }
impl AtomicUsize { pub const unsafe fn from_ptr<'a>(ptr: *mut usize) -> &'a AtomicUsize; }

impl AtomicI8    { pub const unsafe fn from_ptr<'a>(ptr: *mut i8)    -> &'a AtomicI8;    }
impl AtomicI16   { pub const unsafe fn from_ptr<'a>(ptr: *mut i16)   -> &'a AtomicI16;   }
impl AtomicI32   { pub const unsafe fn from_ptr<'a>(ptr: *mut i32)   -> &'a AtomicI32;   }
impl AtomicI64   { pub const unsafe fn from_ptr<'a>(ptr: *mut i64)   -> &'a AtomicI64;   }
impl AtomicIsize { pub const unsafe fn from_ptr<'a>(ptr: *mut isize) -> &'a AtomicIsize; }
```
…=cjgillot

Remove `ResultsVisitable`

`ResultsVisitable` has annoyed me for a while. This PR removes it. Details in the individual commits.

r? `@cjgillot.`
…r=compiler-errors

mark is_val_statically_known intrinsic as stably const-callable

The intrinsic doesn't actually "do" anything in terms of language semantics, and we are already using it in stable const fn. So let's just properly mark it as stably const-callable to avoid needing `rustc_allow_const_fn_unstable` (and thus reducing noise and keeping the remaining `rustc_allow_const_fn_unstable` as a more clear signal).

Cc `@rust-lang/lang` usually you have to approve exposing intrinsics in const, but this intrinsic is basically just a compiler implementation detail. So FCP doesn't seem necessary.
Cc `@rust-lang/wg-const-eval`
…nd-empty-v2, r=notriddle

rustdoc search: allow queries to end in an empty path segment

fixes rust-lang#129707

this can be used to show all items in a module,
or all associated items for a type.
currently sufferes slightly due to case insensitivity, so `Option::` will also show items in the `option::` module.

it disables the checking of the last path element, otherwise only items with short names will be shown

r? `@notriddle`
Unify FnKind between AST visitors and make WalkItemKind more straight forward

Unifying `FnKind` requires a bunch of changes to `WalkItemKind::walk` signature so I'll change them in one go

related to rust-lang#128974

r? `@petrochenkov`
Deny capturing late-bound ty/const params in nested opaques

First, this reverts a7f6095. I can't exactly remember why I approved this specific bit of rust-lang#132466; specifically, I don't know that the purpose of that commit is, and afaict we will never have an opaque that captures late-bound params through a const because opaques can't be used inside of anon consts. Am I missing something `@cjgillot?` Since I can't see a case where this matters, and no tests seem to fail.

The second commit adds a `deny_late_regions: bool` to distinguish `Scope::LateBoundary` which should deny *any* late-bound params or just ty/consts. Then, when resolving opaques we wrap ourselves in a `Scope::LateBoundary { deny_late_regions: false }` so that we deny late-bound ty/const, which fixes a bunch of ICEs that all vaguely look like `impl for<T> Trait<Assoc = impl OtherTrait<T>>`.

I guess this could be achieved other ways; for example, with a different scope kind, or maybe we could just reuse `Scope::Opaque`. But this seems a bit more verbose. I'm open to feedback anyways.

Fixes rust-lang#131535
Fixes rust-lang#131637
Fixes rust-lang#132530

I opted to remove those crashes tests ^ without adding them as regular tests, since they're basically triggering uninteresting late-bound ICEs far off in the trait solver, and the reason that existing tests such as `tests/ui/type-alias-impl-trait/non-lifetime-binder-in-constraint.rs` don't ICE are kinda just coincidental (i.e. due to a missing impl block). I don't really feel motivated to add random permutations to tests just to exercise non-lifetime binders.

r? cjgillot
…r=jieyouxu

Opt out TaKO8Ki from review rotation for now

Hi `@TaKO8Ki,` I'm opting you out from compiler/diagnostics review rotation for now because I *think* you're very busy recently. Please feel free to re-add yourself (or close this PR) whenever you have more time / feel like it.
@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself 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-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. rollup A PR which is a rollup labels Nov 16, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Nov 16, 2024

📌 Commit 17a9a7e has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Nov 16, 2024

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 Nov 16, 2024
@bors
Copy link
Contributor

bors commented Nov 16, 2024

⌛ Testing commit 17a9a7e with merge d924044...

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

Rollup of 7 pull requests

Successful merges:

 - rust-lang#131717 (Stabilize `const_atomic_from_ptr`)
 - rust-lang#132134 (Remove `ResultsVisitable`)
 - rust-lang#132449 (mark is_val_statically_known intrinsic as stably const-callable)
 - rust-lang#132569 (rustdoc search: allow queries to end in an empty path segment)
 - rust-lang#132787 (Unify FnKind between AST visitors and make WalkItemKind more straight forward)
 - rust-lang#132832 (Deny capturing late-bound ty/const params in nested opaques)
 - rust-lang#133097 (Opt out TaKO8Ki from review rotation for now)

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

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

Click to see the possible cause of the failure (guessed by this bot)
test [run-make] tests\run-make\zero-extend-abi-param-passing ... ok
test [run-make] tests\run-make\test-benches ... ok
test [run-make] tests\run-make\share-generics-export-again ... ok
test [run-make] tests\run-make\rustc-crates-on-stable ... ok
Terminate batch job (Y/N)? 
##[error]The operation was canceled.
Post job cleanup.
[command]"C:\Program Files\Git\bin\git.exe" version
git version 2.47.0.windows.1

@bors
Copy link
Contributor

bors commented Nov 17, 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 Nov 17, 2024
@saethlin saethlin added the CI-spurious-fail-msvc CI spurious failure: target env msvc label Nov 17, 2024
@matthiaskrgr
Copy link
Member Author

@bors retry p=9

@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 Nov 17, 2024
@bors
Copy link
Contributor

bors commented Nov 17, 2024

⌛ Testing commit 17a9a7e with merge 1e0df74...

@klensy
Copy link
Contributor

klensy commented Nov 17, 2024

Is that CI-spurious-fail-msvc? I see only hanged process

2024-11-17T01:38:05.1593963Z test [run-make] tests\run-make\test-benches ... ok
2024-11-17T01:38:07.5171465Z test [run-make] tests\run-make\share-generics-export-again ... ok
2024-11-17T01:38:09.4217626Z test [run-make] tests\run-make\rustc-crates-on-stable ... ok
2024-11-17T03:37:01.7263021Z Terminate batch job (Y/N)? 

@bors
Copy link
Contributor

bors commented Nov 17, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 17, 2024
@bors bors merged commit 1e0df74 into rust-lang:master Nov 17, 2024
7 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 17, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#131717 Stabilize const_atomic_from_ptr 02dab0d7939169f0be64e8f8c1f591c3a9a178cd (link)
#132134 Remove ResultsVisitable 6dcebb651eecfba86f8d7bdaeb41a00f46775079 (link)
#132449 mark is_val_statically_known intrinsic as stably const-call… 8bafab88cd2de32e2d168320c148bcb6e0663b27 (link)
#132569 rustdoc search: allow queries to end in an empty path segme… 10ae21d9c6ef20a6e5ced8952a251fd63a98db18 (link)
#132787 Unify FnKind between AST visitors and make WalkItemKind mor… 02fe7b21a330db2ac9034dfef6f20170501bdc31 (link)
#132832 Deny capturing late-bound ty/const params in nested opaques 3cac4a3c0f4512c6e15d8abc7856877e2af0a878 (link)
#133097 Opt out TaKO8Ki from review rotation for now f94ff3ea5222fdf6ddd3f9d6d520291588bc0897 (link)

previous master: ee4a56e353

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 (1e0df74): comparison URL.

Overall result: ❌ regressions - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

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

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.2% [0.1%, 0.3%] 25
Regressions ❌
(secondary)
0.2% [0.1%, 0.4%] 14
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [0.1%, 0.3%] 25

Max RSS (memory usage)

Results (secondary 3.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)
3.7% [2.0%, 5.4%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

Results (secondary -1.9%)

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)
3.6% [2.3%, 4.8%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.7% [-4.7%, -2.0%] 6
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 789.102s -> 787.86s (-0.16%)
Artifact size: 335.46 MiB -> 335.54 MiB (0.02%)

@rustbot rustbot added the perf-regression Performance regression. label Nov 17, 2024
@nnethercote
Copy link
Contributor

@rust-timer build 6dcebb6

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6dcebb6): comparison URL.

Overall result: ❌ regressions - please read the text below

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.2% [0.1%, 0.3%] 23
Regressions ❌
(secondary)
0.2% [0.1%, 0.4%] 13
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [0.1%, 0.3%] 23

Max RSS (memory usage)

Results (primary 4.2%, secondary 2.3%)

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)
4.2% [3.7%, 4.8%] 2
Regressions ❌
(secondary)
4.1% [2.1%, 6.1%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.3% [-3.3%, -3.3%] 1
All ❌✅ (primary) 4.2% [3.7%, 4.8%] 2

Cycles

Results (secondary -0.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)
5.0% [5.0%, 5.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.6% [-2.8%, -2.3%] 3
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 789.102s -> 789.437s (0.04%)
Artifact size: 335.46 MiB -> 335.56 MiB (0.03%)

@nnethercote
Copy link
Contributor

#132134 looks to be responsible for the perf regressions. I will investigate tomorrow.

@nnethercote
Copy link
Contributor

I couldn't find a way to make #132134 any faster, but looking at it closely led to the semi-related #133891 which more than outweighs the #132134 regressions. So, a happy outcome in the end.

@ChrisDenton ChrisDenton added CI-spurious-fail-mingw CI spurious failure: target env mingw and removed CI-spurious-fail-msvc CI spurious failure: target env msvc labels Dec 6, 2024
@matthiaskrgr matthiaskrgr deleted the rollup-4actosy 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
A-meta Area: Issues & PRs about the rust-lang/rust repository itself CI-spurious-fail-mingw CI spurious failure: target env mingw 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-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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.