Add missing normalization for union fields types

[GuillaumeGomez]

Overshadows #106808.
From the experiment #103985.

In short, it allows to use projections as a type for union's fields.

cc @compiler-errors
r? @oli-obk

[compiler-errors]

We already check union fields' copy implementations (and thus their validity) modulo regions, so this is fine imo.

Fixing that seems orthogonal and probably needs a more involved fix.

[oli-obk]

This PR allows

pub trait Identity {
    type Identity;
}

impl<T> Identity for T {
    type Identity = Self;
}

pub type Foo = u8;

pub union Bar {
    pub a: <Foo as Identity>::Identity,
    pub b: u8,
}

to compile. Before this PR, we did not allow projections in union fields' types, as we could not prove that <Foo as Identity>::Identity: Copy without normalizing, which we are doing now. The : Copy check was already being done by ignoring regions, so erasing them doesn't change anything in that regard. Note that this does not affect projections that can't be normalized, only ones that are fully known.

@rfcbot merge

[rfcbot]

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

• @compiler-errors
• @jackh726
• @lcnr
• @nikomatsakis
• @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]

this would ICE if the union has fields which aren't well formed, e.g. something like <T as NotImplemented>::Assoc. Please add a test for this and check whether it currently causes an ICE. If so you can replace this with try_normalize_erasing_regions. I gues it's fine for normalization to ignore region obligations as we should detect these region errors in WF check.

While this also ignore

[GuillaumeGomez]

If you mean:

pub trait Identity {
    type Identity;
}

/*impl<T> Identity for T {
    type Identity = Self;
}*/

pub type Foo = u8;

pub union Bar {
    a:  <Foo as Identity>::Identity,
    b: u8,
}

Then no problem. I'll add a ui test for it though.

[lcnr]

not sure when exactly normalize_erasing_regions ends up failing with an ICE, only know that it does. Maybe also from something like

trait Identity {
    type Identity;
}
trait NotImplemented {}

impl<T: NotImplemented> Identity for T {
    type Identity = Self;
}

type Foo = u8;

union Bar {
    a:  <Foo as Identity>::Identity,
    b: u8,
}

[GuillaumeGomez]

Working too, adding it as a test.

[lcnr]

hmm, alright. If there's a way to ICE here we will just get a bug report, so that seems fine

[lqd]

dis you mean #107872 ?

[compiler-errors]

🤦 yes

[GuillaumeGomez]

That didn't take long. 🤣

[matthiaskrgr]

Showed up as new in my daily ICE-screening ^^

[GuillaumeGomez]

Great job!

[GuillaumeGomez]

Added the test suggested by @lcnr.

[GuillaumeGomez]

And fixed invalid location of the ui test I added.

[GuillaumeGomez]

Added the new UI test suggested by @lcnr.

[rfcbot]

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

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

This will be merged soon.

[oli-obk]

@bors r+

[bors]

📌 Commit 6c63b94 has been approved by oli-obk

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6c63b94 with merge c40919b...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing c40919b to master...

[rust-timer]

Finished benchmarking commit (c40919b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.1%	[-3.1%, -3.1%]	1
Improvements ✅ (secondary)	-2.4%	[-3.0%, -0.8%]	14
All ❌✅ (primary)	-3.1%	[-3.1%, -3.1%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.1%	[1.1%, 1.1%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.1%	[1.1%, 1.1%]	1