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

Fix async closures in CTFE #120950

Merged
merged 2 commits into from
Feb 13, 2024
Merged

Conversation

compiler-errors
Copy link
Member

First commit renames is_coroutine_or_closure into is_closure_like, because is_coroutine_or_closure_or_coroutine_closure seems confusing and long.

Second commit fixes some forgotten cases where we want to handle TyKind::CoroutineClosure the same as closures and coroutines.

The test exercises the change to ValidityVisitor::aggregate_field_path_elem which is the source of #120946, but not the change to UsedParamsNeedSubstVisitor, though I feel like it's not that big of a deal. Let me know if you'd like for me to look into constructing a test for the latter, though I have no idea what it'd look like (we can't assert against TooGeneric anywhere?).

Fixes #120946

r? oli-obk cc @RalfJung

@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 Feb 11, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 11, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

/// These are all represented with the same `ExprKind::Closure` in the AST and HIR,
/// which makes it difficult to distinguish these during def collection. Therefore,
/// we treat them all the same, and code which needs to distinguish them can match
/// or `hir::ClosureKind` or `type_of`.
Closure,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should also become ClosureLike then?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's probably gonna touch a lot of code, let's do that separately

@RalfJung
Copy link
Member

r=me on the compiler/rustc_const_eval/src/interpret/validity.rs changes. I don't understand the code in ensure_monomorphic_enough so I can't comment on that one.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 12, 2024

@bors r=RalfJung,oli-obk

@bors
Copy link
Contributor

bors commented Feb 12, 2024

📌 Commit 8781637 has been approved by RalfJung,oli-obk

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 Feb 12, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 12, 2024
…, r=RalfJung,oli-obk

Fix async closures in CTFE

First commit renames `is_coroutine_or_closure` into `is_closure_like`, because `is_coroutine_or_closure_or_coroutine_closure` seems confusing and long.

Second commit fixes some forgotten cases where we want to handle `TyKind::CoroutineClosure` the same as closures and coroutines.

The test exercises the change to `ValidityVisitor::aggregate_field_path_elem` which is the source of rust-lang#120946, but not the change to `UsedParamsNeedSubstVisitor`, though I feel like it's not that big of a deal. Let me know if you'd like for me to look into constructing a test for the latter, though I have no idea what it'd look like (we can't assert against `TooGeneric` anywhere?).

Fixes rust-lang#120946

r? oli-obk cc `@RalfJung`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 12, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#118983 (Warn on references casting to bigger memory layout)
 - rust-lang#119451 (Gate PR CI on clippy correctness lints)
 - rust-lang#120273 (compiletest: few naive improvements)
 - rust-lang#120889 (Implement Instant for UEFI)
 - rust-lang#120938 (Implement sys/thread for UEFI)
 - rust-lang#120950 (Fix async closures in CTFE)
 - rust-lang#120958 (Dejargonize `subst`)
 - rust-lang#120965 (Add lahfsahf and prfchw target feature)
 - rust-lang#120970 (add another test for promoteds-in-static)
 - rust-lang#120979 (Update books)

Failed merges:

 - rust-lang#120973 (allow static_mut_ref in some tests that specifically test mutable statics)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#118983 (Warn on references casting to bigger memory layout)
 - rust-lang#119451 (Gate PR CI on clippy correctness lints)
 - rust-lang#120273 (compiletest: few naive improvements)
 - rust-lang#120950 (Fix async closures in CTFE)
 - rust-lang#120958 (Dejargonize `subst`)
 - rust-lang#120965 (Add lahfsahf and prfchw target feature)
 - rust-lang#120970 (add another test for promoteds-in-static)
 - rust-lang#120979 (Update books)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 15896bd into rust-lang:master Feb 13, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 13, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2024
Rollup merge of rust-lang#120950 - compiler-errors:miri-async-closurs, r=RalfJung,oli-obk

Fix async closures in CTFE

First commit renames `is_coroutine_or_closure` into `is_closure_like`, because `is_coroutine_or_closure_or_coroutine_closure` seems confusing and long.

Second commit fixes some forgotten cases where we want to handle `TyKind::CoroutineClosure` the same as closures and coroutines.

The test exercises the change to `ValidityVisitor::aggregate_field_path_elem` which is the source of rust-lang#120946, but not the change to `UsedParamsNeedSubstVisitor`, though I feel like it's not that big of a deal. Let me know if you'd like for me to look into constructing a test for the latter, though I have no idea what it'd look like (we can't assert against `TooGeneric` anywhere?).

Fixes rust-lang#120946

r? oli-obk cc ``@RalfJung``
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 26, 2024
…, r=RalfJung,oli-obk

Fix async closures in CTFE

First commit renames `is_coroutine_or_closure` into `is_closure_like`, because `is_coroutine_or_closure_or_coroutine_closure` seems confusing and long.

Second commit fixes some forgotten cases where we want to handle `TyKind::CoroutineClosure` the same as closures and coroutines.

The test exercises the change to `ValidityVisitor::aggregate_field_path_elem` which is the source of rust-lang#120946, but not the change to `UsedParamsNeedSubstVisitor`, though I feel like it's not that big of a deal. Let me know if you'd like for me to look into constructing a test for the latter, though I have no idea what it'd look like (we can't assert against `TooGeneric` anywhere?).

Fixes rust-lang#120946

r? oli-obk cc ``@RalfJung``
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.

ice: miri: aggregate_field_path_elem: got non-aggregate type CoroutineClosure
5 participants