-
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
Fix ICE in ParamConst::find_ty_from_env when handling None values #139333
Fix ICE in ParamConst::find_ty_from_env when handling None values #139333
Conversation
rustbot has assigned @compiler-errors. Use |
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this is the right fix. It is intentional that find_ty_from_env
ICEs if it's run in the wrong ParamEnv
.
I don't see any explanation for what the root cause of the error is. Can you explain the original ICE in detail?
The job Click to see the possible cause of the failure (guessed by this bot)
|
Thank you for your feedback, Michael. You raise an important point about the intentional behavior of The original ICE occurs in the following scenario:
The compiler attempts to find the type associated with the const parameter N during the recursive call The root cause is that during the processing of this recursive call in an async context, the environment doesn't contain the necessary information about the const parameter's type. This is because the async transformation changes how the function is processed, and the const parameter information isn't properly propagated in this specific case. You're right that Rather than modifying the behavior of 1.) Investigate why the const parameter information isn't properly propagated in the async context Would you suggest a different approach to addressing this issue? |
@Aditya-PS-05: Random question, but are you using AI/LLM assistance to generate your PR and comments? |
Unfortunately yes, I have to. I try a lot. But I fail, then I have to use them. |
@compiler-errors , if you can mentor me, how to approach the rust lang code to solve the issue? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been suggested I post a comment. Your approach should be informed by examining the trace of the execution up until it arrives in the situation where it panics. Trying the code on a recent nightly, we get a backtrace from which I have taken this excerpt:
14: 0x74819553b366 - <rustc_middle[6bf438ce1f449cd2]::ty::sty::ParamConst>::find_ty_from_env.cold
15: 0x7481941ee8af - <rustc_trait_selection[6116fba8902975c5]::traits::fulfill::FulfillProcessor as rustc_data_structures[706042bfa2b7a0c2]::obligation_forest::ObligationProcessor>::process_obligation
16: 0x748193807d04 - <rustc_data_structures[706042bfa2b7a0c2]::obligation_forest::ObligationForest<rustc_trait_selection[6116fba8902975c5]::traits::fulfill::PendingPredicateObligation>>::process_obligations::<rustc_trait_selection[6116fba8902975c5]::traits::fulfill::FulfillProcessor>
17: 0x748193d22562 - <rustc_trait_selection[6116fba8902975c5]::traits::fulfill::FulfillmentContext<rustc_infer[cf4ffcedee93cf74]::traits::engine::ScrubbedTraitError> as rustc_infer[cf4ffcedee93cf74]::traits::engine::TraitEngine<rustc_infer[cf4ffcedee93cf74]::traits::engine::ScrubbedTraitError>>::select_where_possible
18: 0x748192f21d56 - <rustc_trait_selection[6116fba8902975c5]::error_reporting::TypeErrCtxt>::error_implies
19: 0x748192ee9742 - <rustc_trait_selection[6116fba8902975c5]::error_reporting::TypeErrCtxt>::report_fulfillment_errors
20: 0x7481939ca35e - <rustc_hir_typeck[75de04720260e6f5]::fn_ctxt::FnCtxt>::try_structurally_resolve_type
21: 0x748194454ef7 - <rustc_hir_typeck[75de04720260e6f5]::fn_ctxt::FnCtxt>::check_expr_with_expectation_and_args
22: 0x748194459c67 - <rustc_hir_typeck[75de04720260e6f5]::fn_ctxt::FnCtxt>::check_expr_with_expectation_and_args
23: 0x74819444fd71 - <rustc_hir_typeck[75de04720260e6f5]::fn_ctxt::FnCtxt>::check_expr_block
24: 0x7481944577c4 - <rustc_hir_typeck[75de04720260e6f5]::fn_ctxt::FnCtxt>::check_expr_with_expectation_and_args
25: 0x748193cc9199 - rustc_hir_typeck[75de04720260e6f5]::check::check_fn
26: 0x748194428532 - <rustc_hir_typeck[75de04720260e6f5]::fn_ctxt::FnCtxt>::check_expr_closure
27: 0x74819445c56c - <rustc_hir_typeck[75de04720260e6f5]::fn_ctxt::FnCtxt>::check_expr_with_expectation_and_args
28: 0x748193cc9199 - rustc_hir_typeck[75de04720260e6f5]::check::check_fn
29: 0x748193cb6091 - rustc_hir_typeck[75de04720260e6f5]::typeck_with_inspect::{closure#0}
30: 0x748193cb4e36 - rustc_query_impl[9fd3fd019d5bb5ba]::plumbing::__rust_begin_short_backtrace::<rustc_query_impl[9fd3fd019d5bb5ba]::query_impl::typeck::dynamic_query::{closure#2}::{closure#0}, rustc_middle[6bf438ce1f449cd2]::query::erase::Erased<[u8; 8usize]>>
31: 0x748193ab65ac - rustc_query_system[e11aa6dc93e86788]::query::plumbing::try_execute_query::<rustc_query_impl[9fd3fd019d5bb5ba]::DynamicConfig<rustc_data_structures[706042bfa2b7a0c2]::vec_cache::VecCache<rustc_span[3f1f0e279447ef10]::def_id::LocalDefId, rustc_middle[6bf438ce1f449cd2]::query::erase::Erased<[u8; 8usize]>, rustc_query_system[e11aa6dc93e86788]::dep_graph::graph::DepNodeIndex>, false, false, false>, rustc_query_impl[9fd3fd019d5bb5ba]::plumbing::QueryCtxt, false>
32: 0x748193ab5f4b - rustc_query_impl[9fd3fd019d5bb5ba]::query_impl::typeck::get_query_non_incr::__rust_end_short_backtrace
33: 0x74819492c1a8 - rustc_hir_analysis[7c975a2d476d1b1d]::collect::type_of::type_of_opaque
We also get this "query stack":
#0 [typeck] type-checking `func`
#1 [type_of_opaque] computing type of opaque `func::{opaque#0}`
Here we can see that we are typechecking an opaque type... the closure I believe. Because the backtrace reports what last happened first, we must read it from the bottom up, unfortunately, to obtain the linear sequence of events. So, starting at 33, we spot the opaque typechecking query that the query stack mentioned. We start scanning upward to find the function which panicked... and then we first reach report_fulfillment_errors
, at 19. We are no longer on a "happy path", but look what happens next, at 17-15?
We are invoking the trait solver again! Probably to find a good suggestion, I'm going to guess. So the problem here is almost certainly... I haven't looked too closely yet... that we are trying to construct a hypothetical answer that would fix the problem for the programmer, maybe a trait they can maybe bring into scope or implement or something, but we are likely passing a dodgy ParamEnv that has been modified or constructed in a way that makes it incapable of answering anything correctly, thus this assert here is discovering, instead, a flaw in the error-reporting code.
Thus, the problem is to find the site where we are doing that and start digging. I suppose I should build the compiler and find line numbers to refine my hypothesis. Or you can do it, as a learning exercise. But you'd have to, in order to solve this, understand either
- how to construct, modify, or find a useful ParamEnv to answer the question we're trying to answer for error-reporting purposes, or
- whether we should give up and just report a different error
You certainly should not take "we ICE, so the primary goal is to stop the ICE" as a given, here, if we are the ones making mistakes in using our own compiler!
Also, probably, the code in the original issue is not fully reduced in terms of complexity and should be simplified further in order to better understand why we even error.
/// Same as `find_const_ty_from_env` but unwraps the result. | ||
/// This will panic if no type is found for the const parameter. | ||
pub fn find_const_ty_from_env_unwrap<'tcx>(self, env: ParamEnv<'tcx>) -> Ty<'tcx> { | ||
self.find_const_ty_from_env(env).unwrap() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the opposite of our usual conventions: we do not qualify the unwrapping function in its name, instead we qualify the Option-returning variant of a function as try
, for a try_something
.
Thanks for the analysis @workingjubilee and I agree that that's generally how I would approach fixing an issue (i.e. root-causing it). However, I think this issue is a lot more subtle and complex than a beginner issue, and I don't think it would be beneficial to mentor a solution to this issue. Thanks for putting up the PR, but I'm going to close it and open a separate solution PR soon, since in the process of root-causing this I think I understand the issue more clearly now, and the fix is kinda subtle. In the future, I would appreciate if you ask AI/LLM to write much smaller and more readable descriptions, or just don't use AI/LLM at all. I find the comment #139333 (comment) to be very overwhelming to process because it's a lot of words that don't say very much about what the issue is, which is common with LLM-generated text. Thanks again for the contribution, and hope you can find an issue to fix that's a bit more beginner friendly! |
Fix ICE in ParamConst::find_ty_from_env when handling None values
This PR fixes issue #139314 where the compiler would panic with "called
Option::unwrap()
on aNone
value" when processing async functions with const parameters.Problem
When processing async functions with const parameters, the compiler would try to find the type of a const parameter by looking through the environment's caller bounds. If no matching type was found, it would call
unwrap()
on aNone
value, causing the compiler to panic.Solution
The fix modifies
ParamConst::find_ty_from_env
to return anOption<Ty<'tcx>>
instead of unwrapping the result directly. A new methodfind_ty_from_env_unwrap
is added for backward compatibility.All call sites are updated to handle the
Option
return type appropriately:rustc_trait_selection/src/traits/fulfill.rs
: Return aProcessResult::Error
when no type is foundrustc_trait_selection/src/solve/fulfill/derive_errors.rs
: Use an error type when no type is foundrustc_trait_selection/src/traits/select/mod.rs
: ReturnEvaluatedToErr
when no type is foundSimilar changes are made to
Placeholder::find_const_ty_from_env
for consistency.Testing
Test cases are added to verify the fix and prevent regression. The compiler now properly reports type errors instead of panicking with an ICE.
Fixes #139314