Skip PhantomData in Unsize checks

[dingxiangfei2009]

This patch proposes relaxation on the type unsizing condition.
PhantomData has been in the current type system exempted from being treated as carrying any data, including dropck, virtual call dispatch by DispatchFromDyn and unsizing container by CoerceUnsize. PhantomData is a special 1-ZST that really carries no "data" of the types it captures.

I propose that we should also extend this interpretation to Unsize.

Back story

This patch falls out of discussion on #148727 and I felt that we are "stricter" on how `Unsize` behaves than necessary.

For #148727, I am looking into anti-fundamenetal trait as suggested, which is a different problem. I don't intend to confuse this as a fix to #148727.

[rustbot]

r? @lcnr

rustbot has assigned @lcnr.
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

[theemathas]

FYI, with the way you currently worded this PR, if it were to be merged, #148727 would be closed.

[theemathas]

This change is visible on stable. The following code fails to compile on nightly, but compiles with this PR

use std::marker::PhantomData;

pub struct Thing<T: ?Sized>(PhantomData<T>, T);

pub fn foo(x: &Thing<i32>) -> &Thing<dyn Send> {
    x
}

I'm unsure whether this code starting to compiling might cause some library to become unsound.

[lcnr]

seems fine from a @rust-lang/types perspective, would like to hand this over to lang.

Is there a specific use enabled by this/what's the reasoning behind this PR?

Right now my vibe here is "sure, would be fine, don't really see why this is relevant" which probably makes it harder for T-lang to make a decision here,

The start of the PR description is a bit confusing 😅

This does not fix #148727, rather probably a good addition to how Unsize behaves. I am looking into anti-fundamenetal trait as suggested to solve a different problem.

While you're explicitly stating that it does not fix #148727, this is the first thing you mention so I end up thinking "hmm, how does this relate to #148727, do I have to understand that issue to review this".

I think this PR literally doesn't interact with that unsoundness, so either don't mention it at all or as a quick note at the end stating that these this PR does not interact with it

[dingxiangfei2009]

@lcnr @theemathas Right, I don't intend to close #148727 with this. I will not mention it.

[BoxyUwU]

This feels very T-types territory to me. Though this change vibes wise seems fine and given both lcnr and I have looked at it that's pretty close to what's required for a types FCP so 🤷‍♀️ If this is going to get a T-types reviewer then handing off to T-lang seems fine to me, though I don't really know what the benefit of doing so is :)

Kinda prefer doing a types FCP here just on principle, though if lang is going to be involved then it's a dual team FCP which is always kinda annoying lol

[BoxyUwU]

Do you have a test where something goes from fail->pass? It's hard to actually tell what this PR changes in terms of user facing impact, there's only a test which has compilation errors even though the description implies we're now being more flexible.

[BoxyUwU]

not normalizing here means that a field such as field: <PhantomData<T> as Identity>::Self will be considered to use T rather than ignoring it like we do in DispatchFromDyn and CoerceUnsized checks

add a test for this case and then fix it

[jackh726]

Dual types+lang FCP seems appropriate here.

From my perspective, there doesn't seem to be any real reason that we can't do this one way or the other in regards to the type system, so the decision really all comes down to "why would we want this from a lang perspective" which is just T-lang.