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

Merged
merged 14 commits into from
Aug 8, 2024
Merged

Rollup of 7 pull requests #128805

merged 14 commits into from
Aug 8, 2024

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Aug 8, 2024

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

compiler-errors and others added 14 commits August 1, 2024 18:46
This includes [1] which means we can remove the (nonworking)
configuration of `no-f16-f128`.

Fixes rust-lang#128401.

[1]: rust-lang/compiler-builtins#652
The default repository server setting has changed on Fuchsia (default is
newly "false"). Now, in order to start the repository server, the config
`repository.server.enabled` must be set to true.
…ce-move, r=BoxyUwU

Skip over args when determining if async-closure's inner coroutine consumes its upvars

rust-lang#125306 implements a strategy for when we have an `async move ||` async-closure that is inferred to be `async FnOnce`, it will force the inner coroutine to also be `move`, since we cannot borrow any upvars from the parent async-closure (since `FnOnce` is not lending):

https://github.com/rust-lang/rust/blob/8e86c9567154dc5a9ada15ab196d23eae2bd7d89/compiler/rustc_hir_typeck/src/upvar.rs#L211-L229

However, when this strategy was implemented, it reused the `ExprUseVisitor` data from visiting the whole coroutine, which includes additional statements due to `async`-specific argument desugaring:

https://github.com/rust-lang/rust/blob/8e86c9567154dc5a9ada15ab196d23eae2bd7d89/compiler/rustc_ast_lowering/src/item.rs#L1197-L1228

Well, it turns out that we don't care about these argument desugaring parameters, because arguments to the async-closure are not the *async-closure*'s captures -- they exist for only one invocation of the closure, and they're always consumed by construction (see the argument desugaring above), so they will force the coroutine's inferred kind to `FnOnce`. (Unless they're `Copy`, because we never consider `Copy` types to be consumed):

https://github.com/rust-lang/rust/blob/8e86c9567154dc5a9ada15ab196d23eae2bd7d89/compiler/rustc_hir_typeck/src/expr_use_visitor.rs#L60-L66

However, since we *were* visiting these arg exprs, this resulted in us too-aggressively applying `move` to the inner coroutine, resulting in regressions. For example, this PR fixes rust-lang#128516. Fascinatingly, the note above about how we never consume `Copy` types is why this only regressed when the argument types weren't all `Copy`.

I tried to leave some comments inline to make this more clear :)
…pos, r=BoxyUwU

Emit an error for invalid use of the `#[no_sanitize]` attribute

fixes rust-lang#128487.

Currently, the use of the `#[no_sanitize]` attribute for Mod, Impl,... is incorrectly permitted. This PR will correct this issue by generating errors, and I've also added some UI test cases for it.

Referenced rust-lang#128458. As far as I know, the `#[no_sanitize]` attribute can only be used with functions, so I changed that part to `Fn` and `Method` using `check_applied_to_fn_or_method`. However, I couldn't find explicit documentation on this, so I could be mistaken...
Update `compiler-builtins` to 0.1.117

This includes [1] which means we can remove the (nonworking) configuration of `no-f16-f128`.

Fixes rust-lang#128401.

[1]: rust-lang/compiler-builtins#652

try-job: dist-various-1
Add -Zmetrics-dir=PATH to save diagnostic metadata to disk

r? ``@estebank``
… r=tmandry

Fuchsia Test Runner: enable ffx repository server

The default repository server setting has changed on Fuchsia (default is newly "false"). Now, in order to start the repository server, the config `repository.server.enabled` must be set to true.
…=petrochenkov

refactor(rustc_expand::mbe): Don't require full ExtCtxt when not necessary

Refactor `mbe::diagnostics::failed_to_match_macro()` to not require a full `ExtCtxt`, but only a `&ParseSess`. It hard-required the `ExtCtxt` only for a call to `cx.trace_macros_diag()`, which we move instead to the only call-site of the function.

Note: This could be a potential change in observed behavior, because a call to `cx.trace_macros_diag()` now always happens after `failed_to_match_macro()` was called, where before it was only called at the end of the main return path of the function. But since `trace_macros_diag()` "flushes" out any not-yet-reported errors, it should be ok to call it for all paths, since there shouldn't be any on the non-main paths I think. However, I don't know the rest of the codebase well enough to say that with 100% confidence, but `tests/ui` still pass, which gives at least some confidence in the change.

Also concretize the return type from `Box<dyn MacResult>` to `(Span, ErrorGuaranteed)`, because this function will _always_ return an error, and never any other kind of result.

Was part of rust-lang#128605 and rust-lang#128747, but is a standalone refactoring.

r? ``@petrochenkov``
…ompiler-errors

Add tracking issue to core-pattern-type

While the actual `pattern_types` feature flag has an issue assigned, the exported macro and its module do not.

cc rust-lang#123646
@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs 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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure 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. rollup A PR which is a rollup labels Aug 8, 2024
@tgross35
Copy link
Contributor Author

tgross35 commented Aug 8, 2024

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Aug 8, 2024

📌 Commit 36b9aee has been approved by tgross35

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

bors commented Aug 8, 2024

⌛ Testing commit 36b9aee with merge 9337f7a...

@bors
Copy link
Contributor

bors commented Aug 8, 2024

☀️ Test successful - checks-actions
Approved by: tgross35
Pushing 9337f7a to master...

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

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#128520 Skip over args when determining if async-closure's inner co… 92d98a5bb4e0619b5fc86392c956d08971e3ecd5 (link)
#128552 Emit an error for invalid use of the #[no_sanitize] attri… d7af83bd78860618d23a93afdc220664c35a76e5 (link)
#128691 Update compiler-builtins to 0.1.117 2e88feec8e37353afab16558fbf02414f990e0ff (link)
#128702 Add -Zmetrics-dir=PATH to save diagnostic metadata to disk 1a147453499e0c31ef2aff0651a36e07c254db0f (link)
#128797 Fuchsia Test Runner: enable ffx repository server 2dac96bd5d6b6a62412964f966927a3a5b14423d (link)
#128798 refactor(rustc_expand::mbe): Don't require full ExtCtxt whe… dbea38e7a794c7a9f8bccb9f494342eb98efcc63 (link)
#128800 Add tracking issue to core-pattern-type 0f72cba64322729ed8de6feebded929656a9adcb (link)

previous master: 0d65e5a180

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 (9337f7a): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

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

Max RSS (memory usage)

Results (primary 2.3%, secondary 1.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)
2.3% [1.2%, 3.6%] 3
Regressions ❌
(secondary)
3.8% [1.5%, 7.5%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.7% [-3.1%, -2.2%] 2
All ❌✅ (primary) 2.3% [1.2%, 3.6%] 3

Cycles

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

Binary size

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

Bootstrap: 761.974s -> 762.874s (0.12%)
Artifact size: 336.94 MiB -> 337.10 MiB (0.05%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs 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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants