Better closure requirement propagation.

[LorrensP-2158466]

Fixes #148289
Fixes #104477
Should unblock #148187

When propagating closure requirements we:

• no longer try to find a post dominiting region for the list of non-local lower bounds of longer_fr.
• we filter the list of longer_fr-: shorter_fr+ to only these constraints between external regions which are required regardless. If no such constraint exists, we fall back to propagating all of them.

See the FCP for more information.

r? @lcnr

[rustbot]

Some changes occurred in src/tools/cargo

cc @ehuss

[LorrensP-2158466]

Excuse me ehuss for the automatic ping, I was a bit too fast with pushing the typo fixes reported by the tidy job.

[lcnr]

@bors try @rust-timer

let's do a crater run, even if I think it should be fine. Then I/we need to write an FCP proposal 😁

[LorrensP-2158466]

let's do a crater run, even if I think it should be fine. Then I/we need to write an FCP proposal 😁

Alright!

[LorrensP-2158466]

Then I/we need to write an FCP proposal 😁

BTW, I would love to help with this if isn't to much to ask. I have never done it, so I might as well get some experience. :)

[rust-bors]

☀️ Try build successful (CI)
Build commit: f27166d (f27166d4c64dc29edc5ea3214357f191edf8b635, parent: f5711a55f5d5e2f942057d0f6d648dd2d8b2c37b)

[lcnr]

@craterbot check

[craterbot]

👌 Experiment pr-148329 created and queued.
🤖 Automatically detected try build f27166d
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[Kobzol]

@rust-timer build f27166d

[rust-timer]

Finished benchmarking commit (f27166d): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

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

Cycles

Results (secondary -4.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.7%	[-5.4%, -4.0%]	2
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 474.205s -> 473.796s (-0.09%)
Artifact size: 390.90 MiB -> 390.90 MiB (0.00%)

[craterbot]

🚧 Experiment pr-148329 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

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

[lcnr]

nits, then r=me

View changes since this review

[lqd]

do you mean the stabilization report, or the new comments in try_propagate_universal_region_error?

[LorrensP-2158466]

Just te pr in general, like the information contained in it.
Is this not really done in tests?

[lqd]

It would be cool to also clean the git history up before merging.

[rustbot]

⚠️ Warning ⚠️

• This PR is based on an upstream commit that is older than 28 days.

  It's recommended to update your branch according to the rustc-dev-guide.

[LorrensP-2158466]

@rustbot ready

then r=me

I don't have r privileges.

[lqd]

And just for completeness' sake, there still are a handful of todos in the stabilization report.

See the tests added in this PR for things which now compile.

If we incorrectly do not propagate any constraints if there are no longer_fr- then the test TODO fails.

If we incorrectly propagate no constraints in case none of them are already
required, i.e. the subset is empty, then the test TODO fails.

[Kobzol]

@bors delegate+

Now you can approve the PR :)

[bors]

✌️ @LorrensP-2158466, you can now approve this pull request!

If @Kobzol told you to "r=me" after making some further change, please make that change, then do @bors r=@Kobzol

[lcnr]

And just for completeness' sake, there still are a handful of todos in the stabilization report.

See the tests added in this PR for things which now compile.
If we incorrectly do not propagate any constraints if there are no longer_fr- then the test TODO fails.
If we incorrectly propagate no constraints in case none of them are already
required, i.e. the subset is empty, then the test TODO fails.

@LorrensP-2158466 i remember u looking into which tests go from pass -> fail for these TODOs? 🤔 can you link to one of them for each of these todos before merging?

[LorrensP-2158466]

@LorrensP-2158466 i remember u looking into which tests go from pass -> fail for these TODOs? 🤔 can you link to one of them for each of these todos before merging?

Do you mean those listed in the zulip thread? #t-types > work on FCP proposal for closure requirements @ 💬
There were a bunch of them, but you also asked:

I would like to have a single specific test which goes from correctly erroring to failing with these changes

And I have never found such a test nor could I create one.