-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
ice: miri: aggregate_field_path_elem: got non-aggregate type CoroutineClosure
#120946
Comments
bit smaller: #![feature(async_closure, noop_waker, async_fn_traits)]
use std::future::Future;
use std::ops::AsyncFnOnce;
use std::pin::pin;
use std::task::*;
pub fn block_on<T>(fut: impl Future<Output = T>) -> T {
let mut fut = pin!(fut);
let ctx = &mut Context::from_waker(Waker::noop());
loop {
match fut.as_mut().poll(ctx) {
Poll::Pending => {}
Poll::Ready(t) => break t,
}
}
}
async fn call_once(f: impl AsyncFnOnce(i32)) {
f(1).await;
}
pub fn main() {
block_on(async {
let b = 2i32;
let mut async_closure = async move |_| {
let b = &b;
};
call_once(async_closure).await;
});
} cc @rust-lang/miri |
Yeah we have special support for Coroutines and Closures in this logic: rust/compiler/rustc_const_eval/src/interpret/validity.rs Lines 239 to 240 in 77f8c3c
And it seems now we have a new similar kind of unnamed ADT that needs similar support. @compiler-errors How similar is it to Coroutines and Closures, can it reuse the exact same logic? In particular, will Maybe it'd make sense to group these in one |
Hm, why is there a function that says "closure or coroutine" that only checks for closures? rust/compiler/rustc_middle/src/ty/util.rs Lines 550 to 552 in c567edd
Or does |
shorter sample from #![feature(async_closure)]
fn async_closure_test(upvar: &str) -> impl async Fn() + '_ {
async move || {
let hello = String::from("hello");
println!("{hello}, {upvar}");
}
}
fn main() {
let _async_closure = async_closure_test("world");
} |
Yeah #120361 didn't add Miri tests so I am not surprised that it is broken. |
They are extremely different (they have different substs, have different layouts, different traits are implemented for them), and it would be very confusing to group them into one
Yes, this was recently done by @bvanjoi and @petrochenkov, I believe. The reason behind this is that they are all expressed in the HIR (and earlier in the AST) as the same construct
Yes, it does. I can make that change to clarify the function behavior. |
Okay. Would be good to document this in the |
…, 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`
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``
…, 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``
Code
Meta
rustc --version --verbose
:Error output
Backtrace
The text was updated successfully, but these errors were encountered: