typeck/type_of: let wfcheck handle generics in opaque types' substs.

[eddyb]

I was working on #70164, and type_of's handling of opaque types seemed to be, by far, the trickiest use of Ty::walk, but I believe it wasn't doing anything (see #57896 (comment) - I suspect, based on glancing at the PR discussion, that an early attempt was kept in, despite becoming just an overcomplicated way to do exactly the same as the previous simple type equality check).

I would've loved to remove ResolvedOpaqueTy (keep the Ty and lose the Substs), but it looks like the MIR borrowck part of the process needs that now, so it would've been added anyway since #57896, even if that PR hadn't happened.

---

In the process, I've moved the remaining substitution validation to wfcheck, which was already handling lifetimes, and kept only delay_span_bugs in type_of, as an insurance policy.

I've added tests for lifetime and const cases, they seem to be checked correctly now.
(and more uniform than they were in #63063 (comment))

However, the quality of the errors is maybe a bit worse, and they don't trigger when there are other errors (not sure if this is due to compilation stop points or something more specific to one opaque type).

r? @nikomatsakis cc @matthewjasper @oli-obk @Aaron1011

[nikomatsakis]

started reading but ran out of time. =) will return. initial comment.

[nikomatsakis]

Pre-existing, but this code is in dire need of some comments.

To start, can we add a comment explaining what is happening here -- I think it is this:

---

Examine the type parameters to the opaque type used in the defining use and make sure that (a) they are only generic parameters from the enclosing scope and (b) each generic parameter is used at most once. For example, given this function:

type Foo<A, B> = impl Debug;

fn f1<T, U>() -> Foo<T, U>

we would be iterating over the substs T, U and checking that they are distinct. We would error for things like -> Foo<T, T> or -> Foo<u32, U>.

[eddyb]

wfcheck is the primary checker and has a nice comment.
This is just a tombstone insurance policy. But I can try to document it if you want.

[nikomatsakis]

I'd still like a comment, even if only points over to the "primary comment"

[nikomatsakis]

In fact, I think pointing out that this is "just" an insurance policy would be quite useful =)

[nikomatsakis]

r=me with nits addressed (I think just a comment)

[nikomatsakis]

This does seem like more confusing output than the old code...

[eddyb]

The difference is when the checks run and which "uses" (mentions, really) are considered "defining".
From the looks of the code, "defining" vs not is an unimplemented part of the feature.
At least now check::wfcheck and type_of are each more uniform and it should be easier to finish the feature, but I'd rather leave that to someone else.

[nikomatsakis]

This error message seems pretty confusing to me. But I guess the old one was pretty bad too so I don't think it needs to block this PR...

[eddyb]

I used the error that used to be only emitted for lifetimes, also for types and consts.

I don't like either error but I didn't want to make a third variant of my own.

[nikomatsakis]

I guess the other types just didn't happen in practice? Else it'd be nice to have a test.

[eddyb]

These are "exported" by the MIR borrowck (cc @matthewjasper) AFAICT, and that results in late-bound lifetimes (in the fn signature, I presume) being ReFree.

This list is minimal, as in I can't remove either ReEarlyBound nor ReFree without tests failing, and adding more would cause less errors (i.e. allowing buggy situations), not more.

[eddyb]

Actually, @matthewjasper, shouldn't ReFree turn back into ReLateBound? Although, I guess the actual concrete type is in terms of the type Foo<...> = impl Trait; generics, so it's not affected by the substitutions back at the point Foo is mentioned.

[matthewjasper]

I don't think that we currently ever go from a placeholder back to a bound region/type.

[eddyb]

@nikomatsakis Hang on, did you see this, or not? GitHub doesn't show it on your original comment, maybe I added it afterwards?

[eddyb]

Hmm, I haven't pushed since, maybe you left the comment on a commit before I had added the comment?

[eddyb]

@bors r=nikomatsakis

[bors]

📌 Commit 8ad149a has been approved by nikomatsakis

[bors]

⌛ Testing commit 8ad149a with merge 49dc2f9...

[bors]

☀️ Test successful - checks-azure
Approved by: nikomatsakis
Pushing 49dc2f9 to master...