GCI: During reachability analysis don't try to evaluate the initializer of overly generic free const items#153269
GCI: During reachability analysis don't try to evaluate the initializer of overly generic free const items#153269fmease wants to merge 1 commit intorust-lang:mainfrom
Conversation
rustbot
added
S-waiting-on-review
|
|
fmease
force-pushed
the
gci-reach-no-eval
branch
from
c5e361a to
7d5e35d
Compare
There was a problem hiding this comment.
Going off on a tangent, the whole "don't eval the initializer for non-lifetime-parametric free const items" thing doesn't apply to mGCA's type-level consts which I'm pretty sure is very intentional (IINM) and thus fine, it's also not GCI-specific, e.g., trait T { type const K: usize = const { panic!(); }; } (Self in scope).
I'm only concerned about const items with bodies anyway since the whole idea is to prevent const eval's "bespoke" handling of "too generic" consts "being user observable" / load-bearing for program correctness (the other motivation being consts should behave like fns in this specific scenario) or rephrased "type based" > "value based".
There was a problem hiding this comment.
yeah type const stuff is whatever here. those should be handled by whatever normal:tm: thing we do for wfchecks of type system stuff
| Err(ErrorHandled::TooGeneric(_)) => self.visit_const_item_rhs(init), | ||
| // We've checked at the start that there aren't any non-lifetime params | ||
| // in scope. The const initializer can't possibly be too generic. | ||
| Err(ErrorHandled::TooGeneric(_)) => bug!(), |
There was a problem hiding this comment.
I'm unsure if this is true 🤔 I would expect it to be possible to get TooGeneric back when there are trivial bounds on the const item which prevent normalization in the const's body.
I guess that's fine for now anyhow... The whole setup here needs to be completely reworked to not evaluate stuff with false bounds or with generic parameters. And once that's done we should start evaluating with TypingEnv::fully_monomorphized which would fix that 🤔
There was a problem hiding this comment.
I intend to get back to that work.... somewhat soon, I think mGCA stuff is finally calming down so I should be able to put more time towards pushing on that again
There was a problem hiding this comment.
You are right, it crashes on the following snippet:
#![feature(generic_const_items)]
pub const X: () = <i32 as Trait<'static>>::K
where
for<'a> i32: Trait<'a>;
trait Trait<'a> { const K: (); }I'm gonna fix this.
I guess that's fine for now anyhow... The whole setup here needs to be completely reworked to not evaluate stuff with false bounds or with generic parameters
For sure! Still, I consider this PR to be a stop-gap since it's a bit janky that pub const _<T>: () = panic!(); panics while const _<T>: () = panic!(); does not.
|
@rustbot author |
rustbot
added
S-waiting-on-author
…er of overly generic free const items
fmease
force-pushed
the
gci-reach-no-eval
branch
from
7d5e35d to
cf4e8c6
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@rustbot review |
rustbot
added
S-waiting-on-review
3 participants
We generally don't want the initializer of free const items to get evaluated if they have any non-lifetime generic parameters. However, while I did account for that in HIR analysis & mono item collection (#136168 & #136429), I didn't account for reachability analysis so far which means that on main we still evaluate such items if they are public for example.
The closed PR #142293 from a year ago did address that as a byproduct but of course it wasn't merged since its primary goal was misguided. This PR extracts & improves upon the relevant parts of that PR which are necessary to fix said issue.
Follow up to #136168 & #136429.
Partially supersedes #142293.
Part of #113521.
r? @BoxyUwU