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 3 pull requests #131404

Merged
merged 15 commits into from
Oct 8, 2024
Merged

Rollup of 3 pull requests #131404

merged 15 commits into from
Oct 8, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

nnethercote and others added 15 commits October 8, 2024 16:27
`OutlivesEnvironment::new` can call `OutlivesEnvironment::with_bounds`
with an empty `extra_bounds`. And once that's done,
`OutlivesEnvironmentBuilder` has a single use and can be inlined and
removed into `OutlivesEnvironment::with_bounds`.
Prompted by rust-lang#131134, which tried to remove `InferOk<'tcx, ()>`
occurrences.
I.e. fixing comments lines that are too long or too short.
It's a weird method, and used weirdly:
- It's on `RegionConstraintCollector` but operates on
  `RegionConstraintStorage`. So at both call sites we create a temporary
  `RegionConstraintCollector`, using `with_log`, to call it.
- It `take`s just two of the six fields in `RegionConstraintStorage`.
  At one of the two call sites we unnecessarily clone the entire
  `RegionConstraintStorage` just to take those two fields.

This commit just inlines and removes it. We no longer need to `take` the
two fields, we can just use them directly.
`LeakCheck` can own `mini_graph` and `rcc` instead of holding references
to them. This requires inlining `assign_scc_value` to avoid a borrowck
error, but that's fine because it has a single call site.
`Deref`/`DerefMut` can be useful, but they can also obfuscate. I don't
think they're worth it for `RegionConstraintCollector`. They're also not
present on the similar types `OpaqueTypeTable` and `TypeVariableTable`.
Sufficient time has passed (> 6 months) since we migrated from `//` to
`//@`, so let's drop the legacy directive check as it causes friction
due to false positives.
…ups, r=lcnr

More `rustc_infer` cleanups

A sequel to rust-lang#131226.

r? `@lcnr`
…heck, r=Urgau

Drop compiletest legacy directive check

Sufficient time has passed (> 6 months) since we migrated from `//` to `//`@`,` so let's drop the
legacy directive check as it causes friction due to false positives.

As a side-effect, dropping the legacy directive check simplifies the directive scanning logic.

The legacy directive check was originally added to help people be aware of the migration.

Blocker for rust-lang#131382 cc `@ehuss.`

Can be reviewed by any compiler/bootstrap reviewer.
@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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. rollup A PR which is a rollup labels Oct 8, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=3

@bors
Copy link
Contributor

bors commented Oct 8, 2024

📌 Commit 37a8ad8 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 Oct 8, 2024
@bors
Copy link
Contributor

bors commented Oct 8, 2024

⌛ Testing commit 37a8ad8 with merge 10a9ee0...

@bors
Copy link
Contributor

bors commented Oct 8, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 8, 2024
@bors bors merged commit 10a9ee0 into rust-lang:master Oct 8, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Oct 8, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#131348 More rustc_infer cleanups 4373263e835de8e67d7168c9e2f061a19bce7858 (link)
#131392 Drop compiletest legacy directive check d061ceb01519f61f44b40540eff9b925264a49ae (link)
#131395 Add a mailmap entry for bjorn3 0778c7a323f830b67a96f85662a697943d752fd5 (link)

previous master: 68e4d9654e

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 (10a9ee0): comparison URL.

Overall result: ❌✅ regressions and improvements - 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
Regressions ❌
(secondary)
2.9% [2.9%, 2.9%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.2% [-0.2%, -0.2%] 4
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (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)
- - 0
Regressions ❌
(secondary)
3.1% [3.1%, 3.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.9% [-1.9%, -1.9%] 2
All ❌✅ (primary) - - 0

Cycles

Results (secondary -2.8%)

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)
- - 0
Improvements ✅
(secondary)
-2.8% [-3.4%, -2.2%] 8
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 775.182s -> 775.795s (0.08%)
Artifact size: 329.46 MiB -> 329.57 MiB (0.04%)

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-testsuite Area: The testsuite used to check the correctness of rustc 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-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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants