Yeet `QueryTypeRelatingDelegate`, move `NllTypeRelating` into borrowck crate - #121321

compiler-errors · 2024-02-20T00:53:15Z

We can just use the existing equate relation for query instantiation. I don't expect us to want to move everything into TypeRelating.

r? lcnr

rustbot · 2024-02-20T00:53:23Z

Type relation code was changed

cc @compiler-errors, @lcnr

lcnr · 2024-02-20T16:35:49Z

 }

-impl<'tcx> TypeRelatingDelegate<'tcx> for NllTypeRelatingDelegate<'_, '_, 'tcx> {
+impl<'tcx> NllTypeRelatingDelegate<'tcx> for NllNllTypeRelatingDelegate<'_, '_, 'tcx> {


this is now the only NllTypeRelatingDelegate in existance, just stop using a delegate and inline this into the relation? That does require moving the whole relation to rustc_borrowck which... does that cause some privacy issues?

Also... NllNllTypeRelatingDelegate looks horrible 😅 maybe just name the struct `TypeRelatingDelegate' or sth, given that it's in borrowck so clearly for nll

i am in generally in favor of this change

NllNll was a typo, lol

I just inlined the whole thing into borrowck

lcnr

two existing nits, then r=me

lcnr · 2024-02-20T18:22:46Z

@bors try @rust-timer queue

bors · 2024-02-20T18:23:55Z

⌛ Trying commit 9b1f9ed with merge 381aeee...

…ng, r=<try> Yeet `QueryTypeRelatingDelegate` We can just use the existing equate relation for query instantiation. I don't expect us to want to move everything into `TypeRelating`. r? lcnr

bors · 2024-02-20T19:52:08Z

☀️ Try build successful - checks-actions
Build commit: 381aeee (381aeee489f72dbb498f2054c43ac9780b9d7829)

rust-timer · 2024-02-20T21:55:47Z

Finished benchmarking commit (381aeee): 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)	1.8%	[1.8%, 1.8%]	1
Regressions ❌ (secondary)	3.3%	[2.6%, 4.3%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.8%	[1.8%, 1.8%]	1

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.255s -> 640.328s (-0.14%)
Artifact size: 308.58 MiB -> 308.55 MiB (-0.01%)

compiler-errors · 2024-02-21T18:42:40Z

-                    .dcx()
-                    .span_bug(self.delegate.span(), format!("unexpected inference var {b:?}"));
-            }
-            // FIXME(invariance): see the related FIXME above.


I don't believe this FIXME makes sense anymore, so I removed it.

compiler-errors · 2024-02-21T19:09:43Z

@bors r=lcnr

bors · 2024-02-21T19:09:46Z

📌 Commit 64d6303 has been approved by lcnr

It is now in the queue for this repository.

bors · 2024-02-21T19:28:59Z

⌛ Testing commit 64d6303 with merge d7bd9cd...

bors · 2024-02-21T21:26:45Z

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

rust-timer · 2024-02-21T22:41:35Z

Finished benchmarking commit (d7bd9cd): 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)	4.4%	[4.4%, 4.4%]	1
Regressions ❌ (secondary)	3.7%	[3.7%, 3.7%]	1
Improvements ✅ (primary)	-3.0%	[-4.0%, -2.1%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.5%	[-4.0%, 4.4%]	3

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: 649.979s -> 649.698s (-0.04%)
Artifact size: 311.00 MiB -> 310.95 MiB (-0.02%)

rustbot assigned lcnr Feb 20, 2024

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 Feb 20, 2024

lcnr reviewed Feb 20, 2024

View reviewed changes

rust-cloud-vms Bot force-pushed the yeet-querytyperelating branch from 45c5586 to 9b1f9ed Compare February 20, 2024 17:45

lcnr reviewed Feb 20, 2024

View reviewed changes

Comment thread compiler/rustc_borrowck/src/type_check/relate_tys.rs Outdated

Comment thread compiler/rustc_borrowck/src/type_check/relate_tys.rs Outdated

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 Feb 20, 2024

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 Feb 20, 2024

compiler-errors added 2 commits February 21, 2024 18:08

Yeet QueryTypeRelatingDelegate

da1e6a8

Make TypeRelating more NLL-specific

6c03033

rust-cloud-vms Bot force-pushed the yeet-querytyperelating branch from 9b1f9ed to 2415d43 Compare February 21, 2024 18:12

Inline NllTypeRelating into its only usage site

64d6303

rust-cloud-vms Bot force-pushed the yeet-querytyperelating branch from 2415d43 to 64d6303 Compare February 21, 2024 18:37

compiler-errors changed the title ~~Yeet QueryTypeRelatingDelegate~~ Yeet QueryTypeRelatingDelegate, move NllTypeRelating into borrowck crate Feb 21, 2024

compiler-errors commented Feb 21, 2024

View reviewed changes

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 Feb 21, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 21, 2024

bors merged commit d7bd9cd into rust-lang:master Feb 21, 2024

rustbot added this to the 1.78.0 milestone Feb 21, 2024

bors mentioned this pull request Feb 21, 2024

change equate for binders to not rely on subtyping #118247

Merged

ShoyuVanilla mentioned this pull request Mar 11, 2024

internal refactoring: cleanup type relations #121159

Open

3 of 4 tasks

matthiaskrgr mentioned this pull request Oct 11, 2024

ICE: WellFormed .. has escaping bound vars, so it cannot be wrapped in a dummy binder. #131535

Closed

matthiaskrgr mentioned this pull request Nov 21, 2024

ICE: unexpected 'SelfCtor' in pattern, expected identifier #133272

Closed

Uh oh!

Conversation

compiler-errors commented Feb 20, 2024

Uh oh!

rustbot commented Feb 20, 2024

Uh oh!

lcnr Feb 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lcnr commented Feb 20, 2024

Uh oh!

This comment has been minimized.

bors commented Feb 20, 2024

Uh oh!

bors commented Feb 20, 2024

Uh oh!

This comment has been minimized.

rust-timer commented Feb 20, 2024

Overall result: no relevant changes - no action needed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Feb 21, 2024

Uh oh!

bors commented Feb 21, 2024

Uh oh!

bors commented Feb 21, 2024

Uh oh!

bors commented Feb 21, 2024

Uh oh!

rust-timer commented Feb 21, 2024

Overall result: no relevant changes - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lcnr Feb 20, 2024 •

edited

Loading