a general type system cleanup #109119

lcnr · 2023-03-14T13:19:47Z

removes the helper functions traits::fully_solve_X as they add more complexity then they are worth. It's confusing which of these helpers should be used in which context.

changes the way we deal with overflow to always add depth in evaluate_predicates_recursively. It may make sense to actually fully transition to not have recursion_depth on obligations but that's probably a bit too much for this PR.

also removes some other small - and imo unnecessary - helpers.

r? types

lcnr · 2023-03-14T13:59:15Z

changes to the recursion depth are scary as they can easily break crates which recurse right until that depth.

@bors try @rust-timer build

bors · 2023-03-14T13:59:25Z

⌛ Trying commit 66ce86eb3b9141650972282a4c568bc2d6f76863 with merge b9d6bd6dc9d9fa058c92b56258df6e48604cc5b7...

lcnr · 2023-03-14T14:02:17Z

@rust-timer queue

it's not build '^^

rustbot · 2023-03-14T14:09:14Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

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

compiler-errors · 2023-03-14T20:19:10Z

compiler/rustc_hir_analysis/src/check/intrinsic.rs

@@ -59,6 +59,7 @@ fn equate_intrinsic_type<'tcx>(
        require_same_types(
            tcx,
            &cause,
+            ty::ParamEnv::empty(), // FIXME: do all intrinsics have an empty param env?


Intrinsics can have where clauses (const_eval_select is the only one at first glance?)

compiler-errors · 2023-03-14T20:23:13Z

compiler/rustc_hir_typeck/src/op.rs

+                // FIXME: This should potentially just add the obligation to the `FnCtxt`
+                let ocx = ObligationCtxt::new(&self.infcx);
+                ocx.register_obligation(obligation);
+                Err(ocx.select_all_or_error())


We probably use these errors eagerly for better diagnostics. It's likely possible to re-create this obligation_for_method in the diagnostics code and re-process it though, or move the diagnostics code into the ObligationCauseCode::BinOp handling branch of note_cause_code or whatever.

compiler/rustc_lint/src/for_loops_over_fallibles.rs

compiler-errors · 2023-03-14T20:31:27Z

compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs


-        let has_impl = self.infcx.predicate_may_hold(&obligation);
+        // FIXME: should this call a `predicate_must_hold` variant instead?


compiler-errors · 2023-03-14T20:34:18Z

@bors try

bors · 2023-03-14T20:34:38Z

⌛ Trying commit 774e3effe1d05b60f5829730be7f7a13abc1be3e with merge 7a89b05fa7ebfbb9dd798a7dbd888c0550beb838...

bors · 2023-03-14T23:28:58Z

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

rust-timer · 2023-03-15T03:34:32Z

Finished benchmarking commit (7a89b05fa7ebfbb9dd798a7dbd888c0550beb838): comparison URL.

Overall result: no relevant changes - 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 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)	1.6%	[1.6%, 1.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.2%	[-4.0%, -2.4%]	2
All ❌✅ (primary)	-	-	0

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.4%	[-5.2%, -3.6%]	4
All ❌✅ (primary)	-	-	0

compiler-errors · 2023-03-15T04:34:59Z

let's see if anything breaks due to the recursion depth changes

@craterbot check

craterbot · 2023-03-15T04:35:04Z

👌 Experiment pr-109119 created and queued.
🤖 Automatically detected try build 7a89b05fa7ebfbb9dd798a7dbd888c0550beb838
🔍 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-03-15T04:36:19Z

🚧 Experiment pr-109119 is now running

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

craterbot · 2023-03-16T02:02:12Z

🎉 Experiment pr-109119 is completed!
📊 29 regressed and 6 fixed (258511 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

lcnr · 2023-03-16T08:33:18Z

the only breakage seems to be https://crates.io/crates/indented-blocks/0.6.0 which has an explicit #[recursion_limit = 8] in its source. I don't think we should care about breaking that.

@rustbot ready

compiler-errors · 2023-03-20T22:01:48Z

r=me rebased

they add more complexity then they are worth. It's confusing which of these helpers should be used in which context.

lcnr · 2023-03-21T13:34:21Z

@bors r+

bors · 2023-03-21T13:34:23Z

📌 Commit b8541eb has been approved by lcnr

It is now in the queue for this repository.

compiler-errors · 2023-03-21T13:44:13Z

@bors r=compiler-errors

bors · 2023-03-21T13:44:15Z

💡 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: Make some traversable types generic over interner #108214

bors · 2023-03-21T13:44:16Z

📌 Commit b8541eb has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2023-03-22T05:33:21Z

⌛ Testing commit b8541eb with merge 9bdb488...

bors · 2023-03-22T08:17:29Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 9bdb488 to master...

rust-timer · 2023-03-22T14:32:38Z

Finished benchmarking commit (9bdb488): 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.6%	[0.5%, 0.8%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.8%	[-1.0%, -0.6%]	4
All ❌✅ (primary)	-	-	0

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.9%	[0.4%, 1.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.8%	[-1.1%, -0.5%]	4
All ❌✅ (primary)	-	-	0

…r-errors a general type system cleanup removes the helper functions `traits::fully_solve_X` as they add more complexity then they are worth. It's confusing which of these helpers should be used in which context. changes the way we deal with overflow to always add depth in `evaluate_predicates_recursively`. It may make sense to actually fully transition to not have `recursion_depth` on obligations but that's probably a bit too much for this PR. also removes some other small - and imo unnecessary - helpers. r? types

rustbot assigned compiler-errors Mar 14, 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. labels Mar 14, 2023

lcnr added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 14, 2023

lcnr changed the title ~~remove some trait solver helpers~~ a general type system cleanup Mar 14, 2023

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 Mar 14, 2023

compiler-errors reviewed Mar 14, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 15, 2023

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 15, 2023

lcnr force-pushed the trait-system-cleanup branch from 774e3ef to 5b7662d Compare March 15, 2023 13:39

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 Mar 16, 2023

lcnr added 3 commits March 21, 2023 09:57

remove some trait solver helpers

791ce0b

they add more complexity then they are worth. It's confusing which of these helpers should be used in which context.

evaluate: improve and fix recursion depth handling

c63861b

use the correct param env

b8541eb

lcnr force-pushed the trait-system-cleanup branch from 5b7662d to b8541eb Compare March 21, 2023 09:05

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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 21, 2023

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

bors merged commit 9bdb488 into rust-lang:master Mar 22, 2023

rustbot added this to the 1.70.0 milestone Mar 22, 2023

xFrednet mentioned this pull request Mar 22, 2023

Bump rust nightly -> 2023-04-20 rust-marker/marker#123

Closed

lcnr deleted the trait-system-cleanup branch March 22, 2023 08:38

matthewjasper mentioned this pull request May 18, 2023

Propagate recursion depth in collect_predicates_for_types #111736

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

a general type system cleanup #109119

a general type system cleanup #109119

lcnr commented Mar 14, 2023 •

edited

Loading

lcnr commented Mar 14, 2023

bors commented Mar 14, 2023

lcnr commented Mar 14, 2023

This comment has been minimized.

rustbot commented Mar 14, 2023

compiler-errors Mar 14, 2023

compiler-errors Mar 14, 2023

compiler-errors Mar 14, 2023

compiler-errors commented Mar 14, 2023

bors commented Mar 14, 2023

bors commented Mar 14, 2023

This comment has been minimized.

rust-timer commented Mar 15, 2023

compiler-errors commented Mar 15, 2023

craterbot commented Mar 15, 2023

craterbot commented Mar 15, 2023

craterbot commented Mar 16, 2023

lcnr commented Mar 16, 2023

compiler-errors commented Mar 20, 2023

lcnr commented Mar 21, 2023

bors commented Mar 21, 2023

compiler-errors commented Mar 21, 2023

bors commented Mar 21, 2023

bors commented Mar 21, 2023

bors commented Mar 22, 2023

bors commented Mar 22, 2023

rust-timer commented Mar 22, 2023


		let has_impl = self.infcx.predicate_may_hold(&obligation);
		// FIXME: should this call a `predicate_must_hold` variant instead?

a general type system cleanup #109119

a general type system cleanup #109119

Conversation

lcnr commented Mar 14, 2023 • edited Loading

lcnr commented Mar 14, 2023

bors commented Mar 14, 2023

lcnr commented Mar 14, 2023

This comment has been minimized.

rustbot commented Mar 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors commented Mar 14, 2023

bors commented Mar 14, 2023

bors commented Mar 14, 2023

This comment has been minimized.

rust-timer commented Mar 15, 2023

Overall result: no relevant changes - no action needed

compiler-errors commented Mar 15, 2023

craterbot commented Mar 15, 2023

craterbot commented Mar 15, 2023

craterbot commented Mar 16, 2023

lcnr commented Mar 16, 2023

compiler-errors commented Mar 20, 2023

lcnr commented Mar 21, 2023

bors commented Mar 21, 2023

compiler-errors commented Mar 21, 2023

bors commented Mar 21, 2023

bors commented Mar 21, 2023

bors commented Mar 22, 2023

bors commented Mar 22, 2023

rust-timer commented Mar 22, 2023

Overall result: no relevant changes - no action needed

lcnr commented Mar 14, 2023 •

edited

Loading