Skip to content

Remove the CoroutineWitness type #144157

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Jul 18, 2025

It's no longer needed, since it always just matches the def id and substs of the Coroutine that it's contained within.

This PR reworks a bit of obligation stalling logic and dtorck constraint behavior, but otherwise it's pretty straightforward.

This luckily was very easy to do because of the setup work in:

r? @lcnr or reassign (e.g. to oli, who probably would also be down to review)

@rustbot rustbot added PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Jul 18, 2025
@compiler-errors
Copy link
Member Author

@bors2 try @rust-timer queue

(and crater when it's ready)

@rust-timer

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Jul 18, 2025

⌛ Trying commit a25d83f with merge 0282016

To cancel the try build, run the command @bors2 try cancel.

rust-bors bot added a commit that referenced this pull request Jul 18, 2025
Remove the `CoroutineWitness` type

It's no longer needed, since it just always the `Coroutine` that it's contained within.

This PR reworks a bit of obligation stalling logic and dtorck constraint behavior, but otherwise it's pretty straightforward.

r? `@lcnr` or reassign (e.g. to oli, who probably would also be down to review)
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 18, 2025
@rust-log-analyzer

This comment has been minimized.

@@ -905,6 +905,7 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {
}
} else {
p!(print_def_path(did, args));
// TODO: Restore witness types?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll probably leave these out. Otherwise we could have query cycles here.

} else {
// TODO: We could still recurse into upvars and the resume here.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely a FIXME for later. I don't think this has any side effect.

Copy link
Contributor

@lcnr lcnr Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should 😁 you could even inline always_drop_component to just return Err(AlwaysRequiresDrop) here

@rust-bors
Copy link

rust-bors bot commented Jul 18, 2025

☀️ Try build successful (CI)
Build commit: 0282016 (028201683a76a5cd349729c6819c46607119c4e8, parent: 8f08b3a32478b8d0507732800ecb548a76e0fd0c)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0282016): comparison URL.

Overall result: ✅ 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
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.2% [-0.3%, -0.2%] 3
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (secondary -3.8%)

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

Cycles

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

Binary size

Results (primary -0.0%, secondary -0.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)
- - 0
Improvements ✅
(primary)
-0.0% [-0.0%, -0.0%] 4
Improvements ✅
(secondary)
-0.3% [-0.7%, -0.1%] 12
All ❌✅ (primary) -0.0% [-0.0%, -0.0%] 4

Bootstrap: 465.311s -> 466.27s (0.21%)
Artifact size: 374.60 MiB -> 374.58 MiB (-0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 18, 2025
@compiler-errors
Copy link
Member Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-144157 created and queued.
🤖 Automatically detected try build 0282016
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 18, 2025
})
} else {
unreachable!(
"tried to assemble `Sized` for coroutine without enabled feature"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does the copy_clone_conditions talk about Sized in the ICE msg xd

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy paste mistake

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still talks about Sized :3

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

general question: do we need to stall on CoroutineClosure as well?

@compiler-errors
Copy link
Member Author

general question: do we need to stall on CoroutineClosure as well?

No, coroutine-closures don't actually really "contain" a witness, and don't use it in their copy/clone and auto trait impls anyways. The fact it stores a witness in its vars is kinda just for convenience for constructing the coroutine type when computing the return type.

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gamer!

r=me, unsure whether to block on #144156. Waiting on crater seems fine, wouldn't expect there to be any issues though :3

@compiler-errors
Copy link
Member Author

I'll land this after #144156, unless you expect that PR to be blocked for some reason.

In that case, I'd prefer if we invert the stack of these and land this PR without the first commit, and then rebase THAT one to use the new logic for checking the drop-ness of the interior types.

@compiler-errors
Copy link
Member Author

I'll take this out of draft for the pings and to mark it as blocked.

@compiler-errors compiler-errors marked this pull request as ready for review July 21, 2025 17:52
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 21, 2025

Some changes occurred in compiler/rustc_sanitizers

cc @rcvalle

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

changes to the core type system

cc @compiler-errors, @lcnr

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in exhaustiveness checking

cc @Nadrieril

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

@compiler-errors
Copy link
Member Author

@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 21, 2025
@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu-llvm-19-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

---- [debuginfo-gdb] tests/debuginfo/function-names.rs stdout ----
NOTE: compiletest thinks it is using GDB version 15001000

error: check directive(s) from `/checkout/tests/debuginfo/function-names.rs` not found in debugger output. errors:
    (function-names.rs:40) `[...]static fn function_names::const_generic_fn_non_int<{CONST#ffa3db4ca1d52dce}>();`
the following subset of check directive(s) was found successfully:
    (function-names.rs:10) `90: static fn function_names::main();`
    (function-names.rs:12) `192: static fn function_names::generic_func<i32>(i32) -> i32;`
    (function-names.rs:16) `160: static fn function_names::GenericStruct<i32, i32>::impl_function<i32, i32>();`
    (function-names.rs:17) `148: static fn function_names::Mod1::TestStruct2::impl_function();`
    (function-names.rs:18) `132: static fn function_names::TestStruct1::impl_function();`
    (function-names.rs:22) `152: static fn function_names::Mod1::{impl#1}::trait_function();`
    (function-names.rs:23) `137: static fn function_names::{impl#1}::trait_function();`
    (function-names.rs:24) `169: static fn function_names::{impl#3}::trait_function<i32>();`
    (function-names.rs:25) `183: static fn function_names::{impl#5}::trait_function3<function_names::TestStruct1>();`
    (function-names.rs:26) `188: static fn function_names::{impl#6}::trait_function<i32, 1>();`
    (function-names.rs:30) `194: static fn function_names::generic_func::{closure#0}<i32>(*mut function_names::generic_func::{closure_env#0}<i32>);`
    (function-names.rs:31) `107: static fn function_names::main::{closure#0}(*mut function_names::main::{closure_env#0});`
    (function-names.rs:32) `162: static fn function_names::{impl#2}::impl_function::{closure#0}<i32, i32>(*mut function_names::{impl#2}::impl_function::{closure_env#0}<i32, i32>);`
    (function-names.rs:39) `200: static fn function_names::const_generic_fn_bool<false>();`
    (function-names.rs:41) `202: static fn function_names::const_generic_fn_signed_int<-7>();`
    (function-names.rs:42) `203: static fn function_names::const_generic_fn_unsigned_int<14>();`
status: exit status: 0
command: PYTHONPATH="/checkout/src/etc" "/usr/bin/gdb" "-quiet" "-batch" "-nx" "-command=/checkout/obj/build/aarch64-unknown-linux-gnu/test/debuginfo/function-names.gdb/function-names.debugger.script"
--- stdout -------------------------------
GNU gdb (Ubuntu 15.1-1ubuntu2) 15.1
Copyright (C) 2024 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "aarch64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word".

File /checkout/tests/debuginfo/function-names.rs:
90: static fn function_names::main();
107: static fn function_names::main::{closure#0}(*mut function_names::main::{closure_env#0});
112: static fn function_names::main::{coroutine#1}(core::pin::Pin<&mut function_names::main::{coroutine_env#1}>, ()) -> core::ops::coroutine::CoroutineState<(), ()>;

File /rustc/FAKE_PREFIX/library/core/src/ptr/mod.rs:
804: static fn core::ptr::drop_in_place<function_names::main::{coroutine_env#1}>(*mut function_names::main::{coroutine_env#1});

File /checkout/tests/debuginfo/function-names.rs:
194: static fn function_names::generic_func::{closure#0}<i32>(*mut function_names::generic_func::{closure_env#0}<i32>);
192: static fn function_names::generic_func<i32>(i32) -> i32;

File /checkout/tests/debuginfo/function-names.rs:
160: static fn function_names::GenericStruct<i32, i32>::impl_function<i32, i32>();
148: static fn function_names::Mod1::TestStruct2::impl_function();
132: static fn function_names::TestStruct1::impl_function();
162: static fn function_names::{impl#2}::impl_function::{closure#0}<i32, i32>(*mut function_names::{impl#2}::impl_function::{closure_env#0}<i32, i32>);

File /checkout/tests/debuginfo/function-names.rs:
152: static fn function_names::Mod1::{impl#1}::trait_function();
137: static fn function_names::{impl#1}::trait_function();
169: static fn function_names::{impl#3}::trait_function<i32>();
183: static fn function_names::{impl#5}::trait_function3<function_names::TestStruct1>();
188: static fn function_names::{impl#6}::trait_function<i32, 1>();

File /checkout/tests/debuginfo/function-names.rs:
194: static fn function_names::generic_func::{closure#0}<i32>(*mut function_names::generic_func::{closure_env#0}<i32>);
107: static fn function_names::main::{closure#0}(*mut function_names::main::{closure_env#0});
162: static fn function_names::{impl#2}::impl_function::{closure#0}<i32, i32>(*mut function_names::{impl#2}::impl_function::{closure_env#0}<i32, i32>);

File /checkout/tests/debuginfo/function-names.rs:
200: static fn function_names::const_generic_fn_bool<false>();
201: static fn function_names::const_generic_fn_non_int<{CONST#a70c39591cb5f53d}>();
202: static fn function_names::const_generic_fn_signed_int<-7>();
203: static fn function_names::const_generic_fn_unsigned_int<14>();
------------------------------------------
stderr: none



Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PG-exploit-mitigations Project group: Exploit mitigations S-blocked Status: Blocked on something else such as an RFC or other implementation work. S-waiting-on-crater Status: Waiting on a crater run to be completed. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler 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. 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.

6 participants