Skip to content

Rollup of 7 pull requests #128805

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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
Collaborator

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
Collaborator

bors commented Aug 8, 2024

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

@bors
Copy link
Collaborator

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%)

# for free to join this conversation on GitHub. Already have an account? # 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