Always check ConstArgHasType even when otherwise ignoring

[khyperia]

fixes #149774

helping @BoxyUwU finish up #150322, this is a simple tweak/finishing-up of that PR.

this is a breaking change that crater detected has some issues with in Boxy's PR, and hence needs a t-types FCP. I can go and help fix those crates if/once the break is signed off on.

[BoxyUwU]

Suggested change

	matches!(o.predicate.kind().skip_binder(),
	matches!(o.predicate.kind().no_bound_vars().unwrap(),

I think this is safer in that we won't silently skip stuff if we unexpectedly have higher ranked const arg has type goals. Instead we'll loudly explode and can fix that 😌

[BoxyUwU]

thx for taking this on for me

[rustbot]

changes to the core type system

cc @lcnr

[rustbot]

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 68 candidates
• Random selection from 13 candidates

[khyperia]

Background Context

First, it's important to note that:

• We do not check that the RHS of free type aliases are WF: type Foo = Vec<[u32]>; is valid
• We do not check trait objects' where clauses: having trait Object<T: Sized> then let x: &dyn Object<[u32]>; is valid
• We do not check WF related things for lifetimes from a for<'a>

This PR does not fundamentally change the above.

Second, in the past, we lowered many things to anon consts. In doing so, we implicitly caused a typecheck of constants to happen, by enforcing that the body of the anon const matched with the type of the anon const.

• https://rustc-dev-guide.rust-lang.org/const-generics.html#anon-consts
• https://rustc-dev-guide.rust-lang.org/const-generics.html#checking-types-of-const-arguments

This typecheck always happend, regardless of if the usage of the anon const was inside a free type alias or trait object reference. In other words, these used to cause compiler errors:

• struct Struct<const N: usize> {}
  type Foo<const B: bool> = Struct<B>;

• trait Object<const N: usize> {}
  fn f<const B: bool>(a: &dyn Object<B>);

However, recently (a few months ago) we changed how some constants are desugared, no longer generating anon consts for them, and instead representing them immediately. This caused consts in these locations to stop being typechecked.

Actual Change

Eventually, we would like to begin wfchecking free type aliases and trait objects. Today is not that day, so instead, this PR is reintroducing typechecking for constants, to minimize the eventual breakage that will be caused by wfchecking type aliases and trait objects.

• Look at wf obligations for free type aliases, discard all of them except ConstArgHasType, and prove those.
• Look at wf obligations for trait objects, discard all of them except ConstArgHasType, and prove those.

Important to note is that ConstArgHasType cannot interact with lifetimes from a for<'a> on stable, because we do not allow generic parameters (including lifetimes) in the types of const generics, so we are free to continue ignoring them.

Crater Breakage

A few crates have already slipped in some "invalid" code, here's a crater run: #150322 (comment)

There are no crater breakages around the trait objects part of this PR.

Here's an example breakage of the free type alias part of this PR:

error: the constant `MODE` is not of type `usize`
  --> src/lfstd.rs:92:1
   |
92 | type AllocScqRing<const MODE: bool> = ScqRing<Box<[CachePadded<AtomicUsize>]>, MODE>;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `usize`, found `bool`
   |
note: required by a const generic parameter in `ScqRing`
  --> src/scq.rs:47:23
   |
47 | pub struct ScqRing<I, const MODE: usize> {
   |                       ^^^^^^^^^^^^^^^^^ required by this const generic parameter in `ScqRing`

Note that even though these type aliases are clearly incorrect, they are still useable by downstream code - this code compiles successfully before this PR, but fails after it:

struct Struct<const N: usize> {}
type Foo<const B: bool> = Struct<B>;
type Bar<const N: usize> = Foo<N>;
let x: Bar<1_usize>;

The breakage is small enough that one person (me) could easily help fix up broken crates. However, the longer we wait, the more breakage will creep in. We've waited a few months already so it's not desperately urgent, but still.

[BoxyUwU]

r? BoxyUwU

[BoxyUwU]

cc @rust-lang/types
@rfcbot merge types

See: #152931 (comment)

[rust-rfcbot]

Team member @BoxyUwU has proposed to merge this. The next step is review by the rest of the tagged team members:

• @BoxyUwU
• @jackh726
• @lcnr
• @oli-obk
• @spastorino

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[lcnr]

reintroducing typechecking for constants,

This PR checks that the type of constant arguments matches the generic parameter instantiated by them when checking that the containing item is well-formed. We do not go back to calling HIR typeck for them. Is that correct?

[khyperia]

This PR checks that the type of constant arguments matches the generic parameter instantiated by them when checking that the containing item is well-formed. We do not go back to calling HIR typeck for them. Is that correct?

doublechecked with boxy (was only 90% sure of exact definitions of words), that is correct!

"most" constants still lower to anonconsts still, it's just very simple stuff that is now directly represented (as of a few months ago), without lowering to an anon const. So, this PR is just covering those simple things that are no longer anonconsts. AFAIK, anonconsts always have been and continue to be HIR typechecked.

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[BoxyUwU]

I think using the normal obligations fn which normalizes stuff should actually be fine. the normalization just means that the resulting ConstArgHasType goals will have the type and const normalized, but those are always going to be const parameters and integer types which normalization does nothing to anyway.

I think unnormalized_obligations is supposed to only be used by the new solver anyhow

[khyperia]

uuh ok! I trust you on this one, I'm not sure of all the implications here.

Changing this moved some errors around in the UI tests (I don't know why/how that could happen), so I've pushed this as a separate commit instead of amending - please review the changes to be sure it's the right thing to do.

[BoxyUwU]

ah.... right yeah I guess we actually need to use unnormalzed obligations or its a breaking change. I assume: type Bar = [(); panic!()]; errors under your PR now but doesn't if we use unnormalized_obligations?

should go back to the previous state and add a comment mentioning we cant use normalized obligations because it would be a breaking change (though I think we should separately crater such breakage)

[khyperia]

tests/ui/type-alias/lack-of-wfcheck.rs, introduced after this PR was made, in #153738 now fails with this PR. Hmm.

error[E0080]: evaluation panicked: explicit panic
  --> /d/rust/tests/ui/type-alias/lack-of-wfcheck.rs:14:23
   |
LL | type Diverging = [(); panic!()]; // `panic!()` diverging
   |                       ^^^^^^^^ evaluation of `Diverging::{constant#0}` failed here

error: constant evaluation is taking a long time
  --> /d/rust/tests/ui/type-alias/lack-of-wfcheck.rs:22:63
   |
LL | type Several<'a> = dyn HasGenericAssocType<Type<'a, String, { loop {} }> = [u8]>;
   |                                                               ^^^^^^^
   |
   = note: this lint makes sure the compiler doesn't get stuck due to infinite loops in const eval.
           If your compilation actually takes a long time, you can safely allow the lint
help: the constant being evaluated
  --> /d/rust/tests/ui/type-alias/lack-of-wfcheck.rs:22:61
   |
LL | type Several<'a> = dyn HasGenericAssocType<Type<'a, String, { loop {} }> = [u8]>;
   |                                                             ^^^^^^^^^^^
   = note: `#[deny(long_running_const_eval)]` on by default

error: aborting due to 2 previous errors

So the RFC comment listing breaking changes wasn't fully complete, as this causes unused diverging type aliases to now fail to compile. I didn't realize that, sorry. Maybe that's ok? Is a new RFC period needed?

[khyperia]

Ah, sorry, ignore me, it only fails with the unnormalized_obligations -> obligations change (see Boxy's review comment above). Leaving it as unnormalized_obligations does not seem to attempt to const eval the diverging things. I think.

[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]

As far as I can tell, there were two crates broken in the crater run - one of which, ranch, was subsequently fixed by AldaronLau/ranch@791e96a . The other, lfqueue, is fixed by the PR I just opened, linked above.