Skip to content

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Aug 13, 2025

#145194 accidentally provided all arguments of the closure to the witness, but the witness only takes the generic parameters of the defining scope:

let interior = Ty::new_coroutine_witness(tcx, expr_def_id.to_def_id(), parent_args);

Fixes #145288

@rustbot
Copy link
Collaborator

rustbot commented Aug 13, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Aug 13, 2025
@lcnr
Copy link
Contributor Author

lcnr commented Aug 13, 2025

r? compiler-errors

@rustbot
Copy link
Collaborator

rustbot commented Aug 13, 2025

compiler-errors is not on the review rotation at the moment.
They may take a while to respond.

@lcnr lcnr force-pushed the coroutine-witness-yikes branch from abde86f to 4d84149 Compare August 13, 2025 12:10
@compiler-errors
Copy link
Member

@bors r+ rollup=never

@bors
Copy link
Collaborator

bors commented Aug 13, 2025

📌 Commit 4d84149 has been approved by compiler-errors

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 13, 2025
@lcnr
Copy link
Contributor Author

lcnr commented Aug 18, 2025

let's check whether this can be rolled up instead. The bors queue is very long.

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Aug 18, 2025
actually provide the correct args to coroutine witnesses
@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 18, 2025
@rust-bors
Copy link

rust-bors bot commented Aug 18, 2025

☀️ Try build successful (CI)
Build commit: e02b440 (e02b440f79527faea218853ba71bbd61974eed15, parent: 425a9c0a0e365c0b8c6cfd00c2ded83a73bed9a0)

@rust-timer

This comment has been minimized.

@lqd
Copy link
Member

lqd commented Aug 18, 2025

We could just bump its priority above rollups :)

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e02b440): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

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

Max RSS (memory usage)

Results (secondary 4.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Cycles

Results (primary -2.7%, secondary -9.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
5.3% [5.3%, 5.3%] 1
Improvements ✅
(primary)
-2.7% [-2.7%, -2.7%] 1
Improvements ✅
(secondary)
-13.0% [-16.7%, -2.2%] 4
All ❌✅ (primary) -2.7% [-2.7%, -2.7%] 1

Binary size

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

Bootstrap: 470.373s -> 470.202s (-0.04%)
Artifact size: 377.75 MiB -> 377.70 MiB (-0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 18, 2025
@lcnr
Copy link
Contributor Author

lcnr commented Aug 19, 2025

the perf impact looks spurious (the only change is in codegen)

@bors rollup=always

bors added a commit that referenced this pull request Aug 19, 2025
[BETA] Revert "Remove the witness type from coroutine args"

fixes #145151 and #145288

we do not revert on nightly as its instead fixed by #145194 and #145338.

See the discussion in https://rust-lang.zulipchat.com/#narrow/channel/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202025-08-14/near/534490313
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Aug 19, 2025
…mpiler-errors

actually provide the correct args to coroutine witnesses

rust-lang#145194 accidentally provided all arguments of the closure to the witness, but the witness only takes the generic parameters of the defining scope: https://github.com/rust-lang/rust/blob/216cdb7b22b637cef75b7225c642cb7587192643/compiler/rustc_hir_typeck/src/closure.rs#L164

Fixes rust-lang#145288
bors added a commit that referenced this pull request Aug 19, 2025
Rollup of 15 pull requests

Successful merges:

 - #145338 (actually provide the correct args to coroutine witnesses)
 - #145429 (Couple of codegen_fn_attrs improvements)
 - #145452 (Do not strip binaries in bootstrap everytime if they are unchanged)
 - #145464 (Stabilize `const_pathbuf_osstring_new` feature)
 - #145474 (Properly recover from parenthesized use-bounds (precise capturing lists) plus small cleanups)
 - #145486 (Fix `unicode_data.rs` mention message)
 - #145490 (Trace some basic I/O operations in bootstrap)
 - #145493 (remove `should_render` in `PrintAttribute` derive)
 - #145500 (Port must_use to the new target checking)
 - #145505 (Simplify span caches)
 - #145510 (Visit and print async_fut local for async drop.)
 - #145511 (Rust build fails on OpenBSD after using file_lock feature)
 - #145532 (resolve: debug for block module)
 - #145533 (Reorder `lto` options from most to least optimizing)
 - #145537 (Do not consider a `T: !Sized` candidate to satisfy a `T: !MetaSized` obligation.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 99de64b into rust-lang:master Aug 19, 2025
11 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 19, 2025
rust-timer added a commit that referenced this pull request Aug 19, 2025
Rollup merge of #145338 - lcnr:coroutine-witness-yikes, r=compiler-errors

actually provide the correct args to coroutine witnesses

#145194 accidentally provided all arguments of the closure to the witness, but the witness only takes the generic parameters of the defining scope: https://github.com/rust-lang/rust/blob/216cdb7b22b637cef75b7225c642cb7587192643/compiler/rustc_hir_typeck/src/closure.rs#L164

Fixes #145288
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Aug 20, 2025
Rollup of 15 pull requests

Successful merges:

 - rust-lang/rust#145338 (actually provide the correct args to coroutine witnesses)
 - rust-lang/rust#145429 (Couple of codegen_fn_attrs improvements)
 - rust-lang/rust#145452 (Do not strip binaries in bootstrap everytime if they are unchanged)
 - rust-lang/rust#145464 (Stabilize `const_pathbuf_osstring_new` feature)
 - rust-lang/rust#145474 (Properly recover from parenthesized use-bounds (precise capturing lists) plus small cleanups)
 - rust-lang/rust#145486 (Fix `unicode_data.rs` mention message)
 - rust-lang/rust#145490 (Trace some basic I/O operations in bootstrap)
 - rust-lang/rust#145493 (remove `should_render` in `PrintAttribute` derive)
 - rust-lang/rust#145500 (Port must_use to the new target checking)
 - rust-lang/rust#145505 (Simplify span caches)
 - rust-lang/rust#145510 (Visit and print async_fut local for async drop.)
 - rust-lang/rust#145511 (Rust build fails on OpenBSD after using file_lock feature)
 - rust-lang/rust#145532 (resolve: debug for block module)
 - rust-lang/rust#145533 (Reorder `lto` options from most to least optimizing)
 - rust-lang/rust#145537 (Do not consider a `T: !Sized` candidate to satisfy a `T: !MetaSized` obligation.)

r? `@ghost`
`@rustbot` modify labels: rollup
@lcnr lcnr deleted the coroutine-witness-yikes branch August 20, 2025 09:18
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Aug 20, 2025
Rollup of 15 pull requests

Successful merges:

 - rust-lang#145338 (actually provide the correct args to coroutine witnesses)
 - rust-lang#145429 (Couple of codegen_fn_attrs improvements)
 - rust-lang#145452 (Do not strip binaries in bootstrap everytime if they are unchanged)
 - rust-lang#145464 (Stabilize `const_pathbuf_osstring_new` feature)
 - rust-lang#145474 (Properly recover from parenthesized use-bounds (precise capturing lists) plus small cleanups)
 - rust-lang#145486 (Fix `unicode_data.rs` mention message)
 - rust-lang#145490 (Trace some basic I/O operations in bootstrap)
 - rust-lang#145493 (remove `should_render` in `PrintAttribute` derive)
 - rust-lang#145500 (Port must_use to the new target checking)
 - rust-lang#145505 (Simplify span caches)
 - rust-lang#145510 (Visit and print async_fut local for async drop.)
 - rust-lang#145511 (Rust build fails on OpenBSD after using file_lock feature)
 - rust-lang#145532 (resolve: debug for block module)
 - rust-lang#145533 (Reorder `lto` options from most to least optimizing)
 - rust-lang#145537 (Do not consider a `T: !Sized` candidate to satisfy a `T: !MetaSized` obligation.)

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Aug 25, 2025
Rollup of 15 pull requests

Successful merges:

 - rust-lang/rust#145338 (actually provide the correct args to coroutine witnesses)
 - rust-lang/rust#145429 (Couple of codegen_fn_attrs improvements)
 - rust-lang/rust#145452 (Do not strip binaries in bootstrap everytime if they are unchanged)
 - rust-lang/rust#145464 (Stabilize `const_pathbuf_osstring_new` feature)
 - rust-lang/rust#145474 (Properly recover from parenthesized use-bounds (precise capturing lists) plus small cleanups)
 - rust-lang/rust#145486 (Fix `unicode_data.rs` mention message)
 - rust-lang/rust#145490 (Trace some basic I/O operations in bootstrap)
 - rust-lang/rust#145493 (remove `should_render` in `PrintAttribute` derive)
 - rust-lang/rust#145500 (Port must_use to the new target checking)
 - rust-lang/rust#145505 (Simplify span caches)
 - rust-lang/rust#145510 (Visit and print async_fut local for async drop.)
 - rust-lang/rust#145511 (Rust build fails on OpenBSD after using file_lock feature)
 - rust-lang/rust#145532 (resolve: debug for block module)
 - rust-lang/rust#145533 (Reorder `lto` options from most to least optimizing)
 - rust-lang/rust#145537 (Do not consider a `T: !Sized` candidate to satisfy a `T: !MetaSized` obligation.)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sqlx-postgres Fails to Build on Latest 1.91.0
8 participants