Remove `PredicateKind::ClosureKind` #118120

compiler-errors · 2023-11-21T04:36:01Z

We don't need the ClosureKind predicate kind -- instead, Fn-family trait goals are left as ambiguous, and we only need to make progress on FnOnce projection goals for inference purposes.

This is similar to how we do confirmation of Fn-family trait and projection goals in the new trait solver, which also doesn't use the ClosureKind predicate.

Some hacky logic is added in the second commit so that we can keep the error messages the same.

rustbot · 2023-11-21T04:36:10Z

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-11-21T04:36:12Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

compiler-errors · 2023-11-21T04:36:18Z

r? types

cc @rust-lang/types

compiler/rustc_smir/src/rustc_smir/mod.rs

compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs

lcnr · 2023-11-21T13:03:27Z

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

+                            candidates.vec.push(ClosureCandidate { is_const });
+                        } else {
+                            candidates.ambiguous = true;
+                        }


this slightly weakens inference, does it? we now stall closure: FnMut if we don't yet know the closure kind while we could now use that goal to infer e.g. the signature of the closure?

No, since we always make progress on a corresponding FnOnce projection that gets emitted besides it, which will always make sure we constrain the signature.

I don't immediately see a case where stalling an FnMut or Fn trait goal itself will cause inference to be affected negatively, as long as we can make progress on the projection goal and make inference progress on the closure's signature.

ah, that's because the only stable usage of the Fn traits is via paren sugar which always results in a projection predicate as well?

Yes, specifically an FnOnce projection goal, which we don't even check the closure kind in (old solver) projection.

This could result in inference changes if you use the unboxed_closure syntax + raw fn_traits + custom FnMut impl with where clauses that affect inference, but that's 1. unstable, and 2. not something we can support in the new trait solver, so I don't feel particularly symathetic to that use-case even if it does exist (which I'm almost certain it does not).

lcnr · 2023-11-21T13:04:10Z

@bors try @rust-timer queue

would like to run crater here in case my understanding is correct.

bors · 2023-11-21T13:05:21Z

⌛ Trying commit 3c03780 with merge a832c07...

Remove `PredicateKind::ClosureKind` We don't need the `ClosureKind` predicate kind -- instead, `Fn`-family trait goals are left as ambiguous, and we only need to make progress on `FnOnce` projection goals for inference purposes. This is similar to how we do confirmation of `Fn`-family trait and projection goals in the new trait solver, which also doesn't use the `ClosureKind` predicate. Some hacky logic is added in the second commit so that we can keep the error messages the same.

bors · 2023-11-21T14:08:20Z

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

bors · 2023-11-21T14:32:12Z

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

lcnr · 2023-11-21T14:38:48Z

alright r=me after rebase and nits then

compiler-errors · 2023-11-21T17:26:20Z

I guess it doesn't hurt to run crater too

@craterbot check

craterbot · 2023-11-21T17:26:31Z

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

rustbot · 2023-11-21T18:36:01Z

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino, @ouz-a

rust-timer · 2023-11-22T08:59:31Z

Finished benchmarking commit (a832c07): comparison URL.

Overall result: ✅ improvements - no 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.

@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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	2
Improvements ✅ (secondary)	-3.7%	[-8.1%, -0.4%]	14
All ❌✅ (primary)	-0.3%	[-0.3%, -0.3%]	2

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)	2.3%	[1.7%, 2.8%]	2
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.1%]	1
Improvements ✅ (secondary)	-2.1%	[-2.1%, -2.1%]	1
All ❌✅ (primary)	-1.1%	[-1.1%, -1.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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.1%	[-5.6%, -2.0%]	9
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 674.399s -> 675.879s (0.22%)
Artifact size: 313.75 MiB -> 313.67 MiB (-0.02%)

compiler-errors · 2023-11-22T18:06:07Z

We can always revert or fix-forward if regressions are detected in crater, so I will land this now.

@bors r=lcnr

bors · 2023-11-22T18:06:10Z

📌 Commit 128feaa has been approved by lcnr

It is now in the queue for this repository.

bors · 2023-11-22T21:09:04Z

⌛ Testing commit 128feaa with merge 1e9dda7...

bors · 2023-11-22T23:22:56Z

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 1e9dda7 to master...

rust-timer · 2023-11-23T05:05:10Z

Finished benchmarking commit (1e9dda7): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-0.3%, -0.2%]	4
Improvements ✅ (secondary)	-3.8%	[-8.1%, -0.5%]	14
All ❌✅ (primary)	-0.2%	[-0.3%, -0.2%]	4

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.6%, -2.6%]	2
Improvements ✅ (secondary)	-0.9%	[-0.9%, -0.9%]	1
All ❌✅ (primary)	-3.1%	[-3.6%, -2.6%]	2

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.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.9%	[-5.4%, -4.6%]	6
All ❌✅ (primary)	1.7%	[1.7%, 1.7%]	1

Binary size

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

Bootstrap: 675.709s -> 675.896s (0.03%)
Artifact size: 313.73 MiB -> 313.66 MiB (-0.02%)

craterbot · 2023-11-26T13:09:41Z

🚧 Experiment pr-118120 is now running

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

craterbot · 2023-11-28T06:30:13Z

🎉 Experiment pr-118120 is completed!
📊 37 regressed and 5 fixed (390818 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

compiler-errors · 2023-11-28T06:39:37Z

@craterbot check crates=https://crater-reports.s3.amazonaws.com/pr-118120/retry-regressed-list.txt p=1

craterbot · 2023-11-28T06:39:41Z

👌 Experiment pr-118120-1 created and queued.
🤖 Automatically detected try build a832c07
⚠️ Try build based on commit 3c03780, but latest commit is 128feaa. Did you forget to make a new try build?
🔍 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 · 2023-11-28T13:00:50Z

🚧 Experiment pr-118120-1 is now running

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

craterbot · 2023-11-28T13:10:27Z

🎉 Experiment pr-118120-1 is completed!
📊 0 regressed and 0 fixed (37 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

rustbot assigned cjgillot Nov 21, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Nov 21, 2023

rustbot added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Nov 21, 2023

rustbot assigned lcnr and unassigned cjgillot Nov 21, 2023

lcnr reviewed Nov 21, 2023

View reviewed changes

compiler/rustc_smir/src/rustc_smir/mod.rs Show resolved Hide resolved

lcnr reviewed Nov 21, 2023

View reviewed changes

compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs Outdated Show resolved Hide resolved

lcnr reviewed Nov 21, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 21, 2023

This comment has been minimized.

Sign in to view

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Nov 21, 2023

compiler-errors added 2 commits November 21, 2023 18:35

Remove ClosureKind predicate kind

93298ee

Restore closure-kind error messages

128feaa

compiler-errors force-pushed the closure-kind branch from 3c03780 to 128feaa Compare November 21, 2023 18:35

compiler-errors mentioned this pull request Nov 22, 2023

Always make progress on FnOnce ClosureKind goals in the old solver #118114

Closed

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Nov 22, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 22, 2023

bors merged commit 1e9dda7 into rust-lang:master Nov 22, 2023

rustbot added this to the 1.76.0 milestone Nov 22, 2023

bors mentioned this pull request Nov 23, 2023

Handle recursion limit for subtype and well-formed predicates #117754

Merged

compiler-errors deleted the closure-kind branch November 23, 2023 01:24

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 28, 2023

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 28, 2023

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Nov 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `PredicateKind::ClosureKind` #118120

Remove `PredicateKind::ClosureKind` #118120

compiler-errors commented Nov 21, 2023 •

edited

Loading

rustbot commented Nov 21, 2023

rustbot commented Nov 21, 2023

compiler-errors commented Nov 21, 2023

lcnr Nov 21, 2023

compiler-errors Nov 21, 2023 •

edited

Loading

lcnr Nov 21, 2023

compiler-errors Nov 21, 2023

compiler-errors Nov 21, 2023

lcnr commented Nov 21, 2023

This comment has been minimized.

bors commented Nov 21, 2023

bors commented Nov 21, 2023

bors commented Nov 21, 2023

This comment has been minimized.

lcnr commented Nov 21, 2023

compiler-errors commented Nov 21, 2023

craterbot commented Nov 21, 2023

rustbot commented Nov 21, 2023

rust-timer commented Nov 22, 2023

compiler-errors commented Nov 22, 2023

bors commented Nov 22, 2023

bors commented Nov 22, 2023

bors commented Nov 22, 2023

rust-timer commented Nov 23, 2023

craterbot commented Nov 26, 2023

craterbot commented Nov 28, 2023

compiler-errors commented Nov 28, 2023

craterbot commented Nov 28, 2023

craterbot commented Nov 28, 2023

craterbot commented Nov 28, 2023

Remove PredicateKind::ClosureKind #118120

Remove PredicateKind::ClosureKind #118120

Conversation

compiler-errors commented Nov 21, 2023 • edited Loading

rustbot commented Nov 21, 2023

rustbot commented Nov 21, 2023

compiler-errors commented Nov 21, 2023

Choose a reason for hiding this comment

compiler-errors Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcnr commented Nov 21, 2023

This comment has been minimized.

bors commented Nov 21, 2023

bors commented Nov 21, 2023

bors commented Nov 21, 2023

This comment has been minimized.

lcnr commented Nov 21, 2023

compiler-errors commented Nov 21, 2023

craterbot commented Nov 21, 2023

rustbot commented Nov 21, 2023

rust-timer commented Nov 22, 2023

Overall result: ✅ improvements - no action needed

compiler-errors commented Nov 22, 2023

bors commented Nov 22, 2023

bors commented Nov 22, 2023

bors commented Nov 22, 2023

rust-timer commented Nov 23, 2023

Overall result: ✅ improvements - no action needed

craterbot commented Nov 26, 2023

craterbot commented Nov 28, 2023

compiler-errors commented Nov 28, 2023

craterbot commented Nov 28, 2023

craterbot commented Nov 28, 2023

craterbot commented Nov 28, 2023

Remove `PredicateKind::ClosureKind` #118120

Remove `PredicateKind::ClosureKind` #118120

compiler-errors commented Nov 21, 2023 •

edited

Loading

compiler-errors Nov 21, 2023 •

edited

Loading