Revert monomorphization for func::{closure#0} #148023
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Some changes occurred in coverage tests. cc @Zalathar |
rustbot added S-waiting-on-review |
|
theemathas added the beta-nominated 
| The job Click to see the possible cause of the failure (guessed by this bot) |
| I managed to fix async-fn-debug-awaitee-field.rs But I have no idea what to do with cold-attribute.rs, @ehuss as original test creator could you please take a look? |
| It just looks like the label needs to change from |
I don't think that's right; |
| You are right of course. I'm not 100% certain, but from what I can tell we need to do something to force the code generation of the async block. I'm not sure what the best way to do that is, but the following seems to work: diff --git a/tests/codegen-llvm/cold-attribute.rs b/tests/codegen-llvm/cold-attribute.rs index 92b4280eaf2..0ddb3ed7c7f 100644 --- a/tests/codegen-llvm/cold-attribute.rs +++ b/tests/codegen-llvm/cold-attribute.rs @@ -12,20 +12,27 @@ #[cold] pub fn free_function() {} -// CHECK-LABEL: ; cold_attribute::async_block +// CHECK-LABEL: ; cold_attribute::async_fn // CHECK-NEXT: Function Attrs: cold {{.*}} #[cold] -pub async fn async_block() { - async fn x(f: impl Future<Output = ()>) { - f.await; +pub async fn async_fn() {} + +pub fn async_block() { + use std::task::{Context, Waker}; + + fn x(f: impl Future<Output = ()>) { + // This is needed to force the code generation of the async block. + let waker = Waker::noop(); + let mut context = Context::from_waker(&waker); + let mut pinned = std::pin::pin!(f); + let _ = pinned.as_mut().poll(&mut context); } x( - // CHECK-LABEL: ; cold_attribute::async_block::{{{{closure}}}}::{{{{closure}}}} + // CHECK-LABEL: ; cold_attribute::async_block::{{{{closure}}}} // CHECK-NEXT: Function Attrs: cold {{.*}} #[cold] async {}, - ) - .await; + ); } pub fn closure() {Does that look reasonable? |
| I don't think we should land this. There is no fundamental regression here. The failure also happens in code calling this function on stable. Now it happens in the definition |
| I don't have a strong opinion here, so I'll follow whatever the team decides. While you're discussing this, I'll try to fix Just ping me if you decide to not close it, so I will continue fighting with test accordingly |
rustbot removed the S-waiting-on-review | Should I maybe notify https://github.com/s0l0ist/leptos-repro about this regression and ask them to increase recursion limit (if this is even a good fix, I'm not sure), or what is usual process here upd: Well this crate is seems abandoned, so I guess they wont update to 1.91 😅 |
cuviper removed the beta-nominated Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
This revert #143290 as per #147976 (comment)
Not sure honestly if this was a cause of regression, can we test it somehow to make sure?
r? @oli-obk