Support DefKind::InlineConst in ConstKind::Unevaluated

[khyperia]

fixes rust-lang/project-const-generics#101

required for rust-lang/project-const-generics#108

consider: Struct<{ (some, stuff, const { hi }) }>. The following is very pseudocode-y, the important parts are whether it says AnonConst or InlineConst, not the Tuple stuff

• On stable, we represent this with: AnonConst(Tuple(some, stuff, InlineConst(hi)))
• Under mGCA, with "direct" arguments, before this PR, it was Tuple(some, stuff, AnonConst(hi)). The inner InlineConst got intercepted in the def_collector with hacks (ConstArgContext) and converted into an AnonConst, even though it has inline const syntax.

It would be nice to keep it as an InlineConst under mGCA, i.e. Tuple(some, stuff, InlineConst(hi)), and have the type system support passing around InlineConsts in ConstKind::Unevaluated (soon to be renamed ConstKind::Alias). This would allow the def collector to not need to know if we are in a "direct" or "regular/anon" context, which it turns out is extremely useful for implementing rust-lang/project-const-generics#108. Supporting InlineConsts in the type system are also useful for other things, for example, mentioned in rust-lang/project-const-generics#101 is arg position const generics experiments.

This PR does two things:

• support InlineConsts in the type system (i.e. in ConstKind::Unevaluated)
• exercise that support, by no longer intercepting mGCA "direct" argument inline consts to be anon consts

r? @BoxyUwU

[rustbot]

HIR ty lowering was modified

cc @fmease

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[BoxyUwU]

did u look through all of DefKind::InlineConst/DefKind::AnonConst or is this just the stuff that when not updated breaks the test suite?

View changes since this review

[BoxyUwU]

all of this can be yeeten i think. this was all just to avoid the with_const_arg call below

[khyperia]

big yeeten! I also refactored this a bit to clean it up now that we don't need to do with_const_arg crimes (probably easier to read the new version in standalone mode, rather than looking at the diff)

[BoxyUwU]

Suggested change

	return;
	self
	.create_def(constant.id, None, DefKind::InlineConst, constant.value.span)
	.def_id()

now that we're not doing a manual with_const_arg call should just be able to let this fall through to the catch all logic at the end of the function

[BoxyUwU]

Suggested change

	let mut visit_alias = || {
	let mut visit_anon_const = || {

[khyperia]

hmm? this is called by TyAlias, AssocTy, Static, Const, AssocConst, AnonConst, and InlineConst, not just AnonConst and InlineConst. (heck if I know a good name for that category of things, though)

[BoxyUwU]

oh nvm then, yeah I didn't see the other bits because of github diff hiding 😅

[rustbot]

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.

[khyperia]

did u look through all of DefKind::InlineConst/DefKind::AnonConst or is this just the stuff that when not updated breaks the test suite?

Looked through them all! Went through them all again just now to be sure. Here's a list of things that might be notable for you (to double check my looking-through-them-all):

• I missed this one!! Added now. The fact it was missing happened to have exactly the same (and correct) behavior as what I changed it to, but it's nice to be explicit.
  

  rust/compiler/rustc_hir_typeck/src/loops.rs

  Line 86 in 1676160

  DefKind::AnonConst => AnonConst,

  • It is currently impossible to hit loops.rs visit_inline_const with a type system inline const, that newly written if statement in my change is "dead" code, but it's nice to be explicit IMO.
    edit: wait no, uuh, type system inline consts are always hir::AnonConst, they can't be hir::ConstBlock, so maybe I should delete the if?

• There's some GCE uses of AnonConst that I have left for now.

• The alias_term_kind_from_def_id vs. unevaluated_const_kind_from_def_id which we discussed in DMs. Maybe I should just add it...

• The FCW cannot use constants which depend on generic parameters in types, I don't think it's possible to get here with the current use case and adding an InlineConst case would be impossible-to-test dead code, so I'm not sure how to properly test it. Maybe I should just blindly add a type system InlineConst case here, I don't know.

• I'm 95% sure this backcompat jank around const fn promotion is fine (the only use of the inline field is that ConstContext::Const allow_const_fn_promotion is later initialized with !inline). I thiiink, because it's only called in MIR stuff, MIR should only see non-type-system InlineConsts.

  rust/compiler/rustc_middle/src/hir/map.rs

  Line 328 in 1676160

  DefKind::InlineConst => BodyOwnerKind::Const { inline: true },

@rustbot ready (except you didn't do rustbot author :P)

[khyperia]

Updated with the stuff I was unsure of in the previous comment.

Also moved a comment that was wonkily placed IMO (due to #158360 )

[Shourya742]

I had a question about the expected invariant here. Does this mean that if we have an InlineConst which is not a type-system inline const, its typeck root is expected to always be an AnonConst?

I was wondering about a nested case where an InlineConst appears inside another InlineConst, and the outer InlineConst is directly under a ConstArg. For example, with the changes here, something like Struct<{ (some, stuff, const { const { hi } }) }> seems like it would be represented roughly as Tuple(some, stuff, InlineConst(InlineConst(hi))).

My understanding is that the outer InlineConst would be the type-system const argument, because it is directly under ConstArg, while the inner InlineConst would not be type-system and would just be an inline const expression inside the outer const body. Now, if we walk from inner to outer, we would stop at the outer inline node, as its the typeck root, but here we would return None which would mean we are not in context arg context I guess? or should nested children of a type-system inline const still be considered to be under the const-arg context? Newbie question, I might be misunderstandIng something.

View changes since the review

[khyperia]

Oh. Yes. You're almost certainly correct this PR is incorrect, but, uuuh, hmm, this currently ICEs on main:

#![feature(min_generic_const_args)]
struct S<const N: usize>;
fn foo<const N: usize>(_: S<{ const { const { N } } }>) {}

[khyperia]

ok, got a repro that doesn't ICE on main, that fails with the code in this PR before a fix (due to the issue you pointed out), that succeeds after my latest push. Added that repro as a test in my latest push as well. Thanks for pointing this out!

I think the ICE on main is totally unrelated, will look into it later.