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

Do check_coroutine_obligations once per typeck root #123993

Merged
merged 4 commits into from
Apr 23, 2024

Conversation

compiler-errors
Copy link
Member

We only need to do check_coroutine_obligations once per typeck root, especially since the new solver can't really (easily) associate which obligations correspond to which coroutines.

This requires us to move the checks for sized coroutine fields into mir_coroutine_witnesses, but that's fine imo.

r? lcnr

@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. labels Apr 15, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-log-analyzer

This comment has been minimized.

@@ -759,7 +759,9 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
tcx.hir().par_body_owners(|def_id| {
if tcx.is_coroutine(def_id.to_def_id()) {
tcx.ensure().mir_coroutine_witnesses(def_id);
tcx.ensure().check_coroutine_obligations(def_id);
tcx.ensure().check_coroutine_obligations(
Copy link
Member Author

Choose a reason for hiding this comment

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

Only thing I'm kinda unsatisfied with is calling check_coroutine_obligations once per coroutine rather than once per typeck root. Not sure if there's a better place to relocate this check, tho.

@lcnr
Copy link
Contributor

lcnr commented Apr 23, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 23, 2024

📌 Commit a8c9a0b has been approved by lcnr

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 Apr 23, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 23, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#120929 (Wrap dyn type with parentheses in suggestion)
 - rust-lang#122591 (Suggest using type args directly instead of equality constraint)
 - rust-lang#122598 (deref patterns: lower deref patterns to MIR)
 - rust-lang#123048 (alloc::Layout: explicitly document size invariant on the type level)
 - rust-lang#123993 (Do `check_coroutine_obligations` once per typeck root)
 - rust-lang#124218 (Allow nesting subdiagnostics in #[derive(Subdiagnostic)])
 - rust-lang#124285 (Mark ``@RUSTC_BUILTIN`` search path usage as unstable)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e15d6f9 into rust-lang:master Apr 23, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 23, 2024
Rollup merge of rust-lang#123993 - compiler-errors:coroutine-obl, r=lcnr

Do `check_coroutine_obligations` once per typeck root

We only need to do `check_coroutine_obligations` once per typeck root, especially since the new solver can't really (easily) associate which obligations correspond to which coroutines.

This requires us to move the checks for sized coroutine fields into `mir_coroutine_witnesses`, but that's fine imo.

r? lcnr
@compiler-errors compiler-errors deleted the coroutine-obl branch April 23, 2024 18:31
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants