do not allow inference in predicate_must_hold (alternative approach)

[compiler-errors]

See the FCP description for more info, but tl;dr is that we should not return EvaluatedToOkModuloRegions if an obligation may hold only with some choice of inference vars being constrained.

Attempts to solve this in the approach laid out by lcnr here: #109558 (comment), rather than by eagerly replacing infer vars with placeholders which is a bit too restrictive.

r? @ghost

[compiler-errors]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 0779b6620d33449d376a8b437caa060a364cd6e0 with merge dc2620d92af7fa332c95a7cd35393bf9510ac125...

[compiler-errors]

@bors try

[bors]

⌛ Trying commit 4230620b72959d6be0cfe37de66d63cc571c5ad2 with merge f1594433169cca61dd6137e11b86ca894968fb2f...

[bors]

☀️ Try build successful - checks-actions
Build commit: f1594433169cca61dd6137e11b86ca894968fb2f (f1594433169cca61dd6137e11b86ca894968fb2f)

[compiler-errors]

@craterbot check

[craterbot]

👌 Experiment pr-110100 created and queued.
🤖 Automatically detected try build f1594433169cca61dd6137e11b86ca894968fb2f
🔍 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

[craterbot]

🚧 Experiment pr-110100 is now running

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

[rust-timer]

Finished benchmarking commit (f1594433169cca61dd6137e11b86ca894968fb2f): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.2%	[0.5%, 1.6%]	7
Regressions ❌ (secondary)	0.3%	[0.3%, 0.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.2%	[0.5%, 1.6%]	7

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)	4.1%	[4.1%, 4.1%]	1
Improvements ✅ (primary)	-2.0%	[-2.0%, -2.0%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.0%	[-2.0%, -2.0%]	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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.6%	[-11.1%, -2.9%]	4
All ❌✅ (primary)	-	-	0

[craterbot]

🎉 Experiment pr-110100 is completed!
📊 28 regressed and 8 fixed (260976 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[rfcbot]

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

[bors]

☔ The latest upstream changes (presumably #109732) made this pull request unmergeable. Please resolve the merge conflicts.

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

[jackh726]

@bors r+ rollup=never

[bors]

📌 Commit c06e611 has been approved by jackh726

It is now in the queue for this repository.

[bors]

⌛ Testing commit c06e611 with merge 19ca569...

[bors]

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing 19ca569 to master...

[rust-timer]

Finished benchmarking commit (19ca569): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

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)	5.8%	[5.8%, 5.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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

Bootstrap: 641.663s -> 641.09s (-0.09%)

[lqd]

Correct me if I'm wrong, but the perf hit looks expected, and since this PR looks like this is a correctness fix, it's deemed acceptable.

[pnkfelix]

Hey @compiler-errors and @lcnr , based on https://github.com/rust-lang/rust/pull/110100/files#diff-cacebb74c41e0865cc830e29d5404a2c69a9901c37e4f1b579eb383ed71b12c8 going from run-pass to compile-fail, this seems like it could be a breaking change, ...

... but I don't see any crater run showing me that I need not worry ...

... nor a link to an existing issue describing what is being changed here (i.e. what issues are being fixed by this change)...

... and the description here refers to some T-types FCP but I have no idea where that is...

(Update: The things I was asking for were on this PR, just hidden under the fold.)

[pnkfelix]

Correct me if I'm wrong, but the perf hit looks expected, and since this PR looks like this is a correctness fix, it's deemed acceptable.

I am going to assume from context (namely the 👍 from @lcnr ) that this is indeed a correctness fix of some form, and therefore the perf regression is expected and acceptable.

@rustbot label: +perf-regression-triaged

[compiler-errors]

@pnkfelix look in the comments above for crater, FCP, and description of changes.

[pnkfelix]

sorry for the noise!