fix ICE in generic_const_parameter_types with inherents

[khyperia]

tracking issue: #137626

relevant PR where the code was added: #154853 (fyi ping @lapla-cogito - nws that this was buggy, it's extreeeemely subtle and easy to miss! ❤️ I mean, I also reviewed that PR and missed it too :3 )

discovered when implementing a change that explicitly tracks whether the args for inherent associated consts are in "self form" or "impl form"

Following along the test case:

• normalize_canonicalized_inherent_projection is called with AliasTermKind::InherentConst with the generic args being in "self form", i.e. [ThreeTypes<u8, u16, u32>]
• traits::normalize_inherent_projection is called with said alias
  • it calls compute_inherent_assoc_term_args, which does the dance of generating fresh vars for each param in the impl block, equating with the self type, and returning what the fresh vars solved to. This converts from "self args" to "impl args", i.e. [u8, u16, u32]
  • it then calls const_of_item and instantiates with [u8, u16, u32]. this is correct and good, const_of_item expects "impl form" args.
  • it then calls push_const_arg_has_type_obligation
    • which calls type_of and instantiates with [u8, u16, u32] to fetch the type of the const, to be able to register a ConstArgHasType. this is correct and good, type_of expects "impl form" args.
  • traits::normalize_inherent_projection returns, dropping the impl form args it computed
• normalize_canonicalized_inherent_projection calls ocx.register_obligations(const_arg_has_type_obligation(...)), passing goal. Remember that goal has the original "self args" generic arg format.
  • const_arg_has_type_obligation calls type_of and instantiates with [ThreeTypes<u8, u16, u32>]. This is no good very bad!! type_of expects "impl form" args, not "self form"!!
  • ICE!! type parameter T3/#2 (T3/#2/2) out of range when instantiating, args=[ThreeTypes<u8, u16, u32>]

The reason I filed this under feature(generic_const_parameter_types) is because for this bug to manifest, type_of must return a type that actually references a generic param to be able to trigger an ICE. Otherwise, the buggy incorrect args are silently ignored and compilation continues "fine".

The fix:

normalize_inherent_projection already registers a ConstArgHasType. why are we doing it a second time. just delete it. 💀

r? @BoxyUwU

[rustbot]

BoxyUwU is currently at their maximum review capacity.
They may take a while to respond.

[BoxyUwU]

Can you check whether the function for normalizing trait projections also checks this twice? If so that should be yeet too ✨ But yeah this PR makes sense to me 👍

View changes since this review

[khyperia]

Can you check whether the function for normalizing trait projections also checks this twice? If so that should be yeet too ✨ But yeah this PR makes sense to me 👍

ough, I did go looking for it when I wrote this PR and didn't find it, but yes, it also happens. It's very deep within the project machinery though, and it only happens for ImplSource::UserDefined (in confirm_impl_candidate), not ImplSource::Builtin or ProjectionCandidate::{ParamEnv, Object, TraitDef}, but that's maaaaybe fine? I'm unsure if "unsoundness possible due to badly defining your own lang items" is like, a valid bug. Anyway, TBH I'm a little more scared of yeeting this for trait projections though... relying on the const value matching the type of the impl implying that it also matches the type of the trait decl feels like it could go ICEy in error scenarios (the ConstArgHasType inside confirm_impl_candidate checks the type on the impl, not the trait). Is prooobably fine though?

I'll yeet it for now and then when you rereview and see this comment you can decide~