hir_expand/static_borrow: Handle Defer and const-generic params in lifted statics#411
Open
rautyrauty wants to merge 2 commits into
Open
hir_expand/static_borrow: Handle Defer and const-generic params in lifted statics#411rautyrauty wants to merge 2 commits into
rautyrauty wants to merge 2 commits into
Conversation
This is necessary because the evaluator throws Defer when a dependency isn't resolved yet, but the SBC call sites had no try/catch, so a deferred eval propagated to std::terminate and aborted libcore on aarch64 once an earlier compare fix let the pass reach those cases. Handling Defer here mirrors what Constant Evaluate already does for the same situation.
…const-generic This is necessary because extract_node's visitor doesn't enumerate every HIR construct that can carry a const-generic value-param reference; ARM Neon's core_arch::arm_shared::neon::generated lifted const blocks (vshl<const N: i32> and friends) end up with _0 = Constant(N/*M:0*/) in the lifted static's MIR with the method-scope binding never remapped, then assert in MonomorphiserPP::get_value because params_def had been wiped to concrete. Until the visitor is exhaustively audited, suppress the wipe whenever the surrounding impl/item has any const-generic value-param -- it just defers the constant evaluation to Trans Monomorph, which already handles genuinely generic statics.
Comment on lines
+913
to
+925
| // `is_generic` is set by the targeted visit overrides above | ||
| // (ExprNode_ConstParam, ExprNode_ArraySized) plus visit_type / | ||
| // visit_path_params. Those reliably cover type- and lifetime- | ||
| // generic references, but a const-generic value param can still | ||
| // leak through paths not yet enumerated -- e.g. ARM Neon's | ||
| // `core_arch::arm_shared::neon::generated` lifted const blocks | ||
| // hit `_ = Constant(N/*M:0*/)` in MIR with the surrounding | ||
| // method's `const N: i32` never remapped, then assert in | ||
| // MonomorphiserPP::get_value because the lifted static was | ||
| // wiped to a concrete (empty params) item. Preserve the params | ||
| // whenever the source context carries any const-generic value | ||
| // param: the only cost is a slightly later (Trans Monomorph) | ||
| // evaluation instead of an SBC-time concrete one. |
Owner
There was a problem hiding this comment.
This is a really verbose comment, and points to a different error. The visitor above should be able to detect that const-generic and flag the generic-ness.
How is that const-generic still getting though without being flagged?
thepowersgang
requested changes
May 24, 2026
thepowersgang
left a comment
thepowersgang
left a comment
Owner
There was a problem hiding this comment.
The catching is good, but the logic for disabling lifting when there are value-generics in scope is questionable.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Two robustness fixes to the Static Borrow Constants pass in
src/hir_expand/static_borrow_constants.cpp, both surfaced while bootstrapping rustc 1.90.0 libcore on aarch64. The pass extracts ad-hoc constant borrows into freshly generated statics; both cases failed once it actually reached generic-bearing inputs (a side effect of #410, the ConstGeneric_Unevaluated::ord fixes).Catch
HIR::Deferfrom the extracted-static const-eval. The evaluator throwsDeferwhen a dependency isn't resolved yet, but the SBC call sites had no try/catch — so a deferred eval propagated tostd::terminate. Handle it here the same wayConstant Evaluatealready does: leave the new static's value as not-yet-generated and let a later pass retry. This required promoting the previously anonymous-namespaceDeferintonamespace HIRso out-of-module passes can catch it.Keep the lifted static generic when the source has a const-generic param.
extract_node's visitor doesn't enumerate every HIR construct that can carry a const-generic value-param reference. ARM Neon'score_arch::arm_shared::neon::generatedlifted const blocks (vshl<const N: i32>and friends) end up with_0 = Constant(N/*M:0*/)in the lifted static's MIR with the method-scope binding never remapped, then assert inMonomorphiserPP::get_valuebecauseparams_defhad been wiped to a concrete static. Until the visitor is exhaustively audited, suppress the wipe-to-concrete optimisation whenever the surrounding impl/item has any const-generic value-param — the constant evaluation is then handled at Trans Monomorph instead of SBC, which is already the path for genuinely generic statics.