implied bounds: explicitly state which types are assumed to be wf

[lcnr]

Adds a new query which maps each definition to the types which that definition assumes to be well formed. The intent is to make it easier to reason about implied bounds.

This change should not influence the user-facing behavior of rustc. Notably, borrowck still only assumes that the function signature of associated functions is well formed while wfcheck assumes that the both the function signature and the impl trait ref is well formed. Not sure if that by itself can trigger UB or whether it's just annoying.

As a next step, we can add WellFormed predicates to predicates_of of these items and can stop adding the wf bounds at each place which uses them. I also intend to move the computation from assumed_wf_types to implied_bounds into the param_env computation. This requires me to take a deeper look at compare_predicate_entailment which is currently somewhat weird wrt implied bounds so I am not touching this here.

r? @nikomatsakis

[nikomatsakis]

r=me with a helper function

[nikomatsakis]

can we make a helper function for this repeated code?

[nikomatsakis]

also here :)

[jackh726]

👍 I'm generally in favor of cleaning up how we calculate wf and implied bounds. It's a bit of mess and confusing in some places.

[lcnr]

@bors r+ rollup=never

[bors]

📌 Commit 736288f has been approved by lcnr

It is now in the queue for this repository.

[lcnr]

@bors r=nikomatsakis

[bors]

💡 This pull request was already approved, no need to approve it again.

• There's another pull request that is currently being tested, blocking this pull request: Rollup of 15 pull requests #100677

[bors]

📌 Commit 736288f has been approved by nikomatsakis

It is now in the queue for this repository.

[bors]

⌛ Testing commit 736288f with merge a9bb589...

[bors]

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing a9bb589 to master...

[rust-timer]

Finished benchmarking commit (a9bb589): comparison url.

Instruction count

• Primary benchmarks: ❌ relevant regressions found
• Secondary benchmarks: ❌ relevant regressions found

	mean1	max	count2
Regressions ❌ (primary)	0.3%	0.5%	22
Regressions ❌ (secondary)	0.4%	0.8%	22
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	0.5%	22

Max RSS (memory usage)

Results

• Primary benchmarks: ❌ relevant regressions found
• Secondary benchmarks: ✅ relevant improvements found

	mean1	max	count2
Regressions ❌ (primary)	2.0%	3.3%	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.4%	-3.5%	5
All ❌✅ (primary)	2.0%	3.3%	2

Cycles

Results

• Primary benchmarks: ❌ relevant regression found
• Secondary benchmarks: ✅ relevant improvement found

	mean1	max	count2
Regressions ❌ (primary)	1.7%	1.7%	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.7%	-2.7%	1
All ❌✅ (primary)	1.7%	1.7%	1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Footnotes

1. the arithmetic mean of the percent change ↩ ↩² ↩³

2. number of relevant changes ↩ ↩² ↩³

[lcnr]

this does actually change user visible behavior. We are now adding implied bounds for unnormalized types during wfcheck. I expect that this is the case of the perf regression. This means that the following now compiles:

struct Foo<T>(T);

trait GoodBye {
    type Forget;
}
impl<T> GoodBye for T {
    type Forget = ();
}

trait NeedsWf<'a, 'b> {
    type Assoc;
}

impl<'a, 'b> NeedsWf<'a, 'b> for Foo<<&'a &'b () as GoodBye>::Forget> {
    type Assoc = &'a &'b ();
}

fn main() {}

When using an impl, we don't actually check wf for these types before normalization, so while this change is desirable, it is currently unsound. I want to avoid reverting this PR and fixing this should be straightforward. Am on vacation right now, going to open an issue for this and intend to fix that once I am back. The fix will be fairly small so 2 weeks until the next beta-cutoff should be enough.

[pnkfelix]

Nominating for discussion at T-compiler meeting tomorrow.

My gut tells me that we should revert this (primarily due to injection of unsoundness; secondarily due to it causing small-but-widespread performance regression).

(If we were to revert it, the intention would be to encourage lcnr to re-land a fixed version of it when they are back from vacation.)

That, or someone else should put effort into putting in a fix in the meantime.

---

Note: I do understand the argument that lcnr is making; namely, that the branch of beta from nightly is on September 16 2022 and so lcnr would have a little over a week to land a fix.

I just don't agree with the above argument. We should be very willing to do reverts in reaction to scenarios like this, unless there is immediate need for this change from T-types.

@rustbot label: I-compiler-nominated

(In addition to nominating, I also opened up an associated zulip thread)

[lcnr]

opened #100989 which reverts the behavior changes and (probably) the perf regression of this PR.