Introduce `trait DebugWithInfcx` to debug format types with universe info #112984

BoxyUwU · 2023-06-23T22:34:46Z

Seeing universes of infer vars is valuable for debugging but currently we have no way of easily debug formatting a type with the universes of all the infer vars shown. In the future I hope to augment the new solver's proof tree output with a DebugWithInfcx impl so that it can show universes but I left that out of this PR as it would be non trivial and this is already large and complex enough.

The goal here is to make the various abstractions taking T: Debug able to use the codepath for printing out universes, that way we can do debug!("{:?}", my_x) and have my_x have universes shown, same for the write! macro. It's not possible to put the Infcx: InferCtxtLike<I> into the formatter argument to Debug::fmt so it has to go into the self ty. For this we introduce the type OptWithInfcx<I: Interner, Infcx: InferCtxtLike<I>, T> which has the data T optionally coupled with the infcx (more on why it's optional later).

Because of coherence/orphan rules it's not possible to write the impl Debug for OptWithInfcx<..., MyType> when OptWithInfcx is in a upstream crate. This necessitates a blanket impl in the crate defining OptWithInfcx like so: impl<T: DebugWithInfcx> Debug for OptWithInfcx<..., T>. It is not intended for people to manually call DebugWithInfcx::fmt, the Debug impl for OptWithInfcx should be preferred.

The infcx has to be optional in OptWithInfcx as otherwise we would end up with a large amount of code duplication. Almost all types that want to be used with OptWithInfcx do not themselves need access to the infcx so if we were to not optional we would end up with large Debug and DebugWithInfcx impls that were practically identical other than that when formatting their fields we wrap the field in OptWithInfcx instead of formatting it alone.

The only types that need access to the infcx themselves are ty/const/region infer vars, everything else is implemented by having the Debug impl defer to OptWithInfcx with no infcx available. The DebugWithInfcx impl is pretty much just the standard Debug impl except that instead of recursively formatting fields with write!(f, "{x:?}") we must do write!(f, "{:?}", opt_infcx.wrap(x)). This is some pretty rough boilerplate but I could not think of an alternative unfortunately.

OptWithInfcx::wrap is an eager Option::map because 99% of callsites were discarding the existing data in OptWithInfcx and did not need lazy evaluation.

A trait InferCtxtLike was added instead of using InferCtxt<'tcx> as we need to implement DebugWithInfcx for types living in rustc_type_ir which are generic over an interner and do not have access to InferCtxt since it lives in rustc_infer. Additionally I suspect that adding universe info to new solver proof tree output will require an implementation of InferCtxtLike for something that is not an InferCtxt although this is not the primary motivaton.

To summarize:

There is a type OptWithInfcx which bundles some data optionally with an infcx with allows us to pass an infcx into a Debug impl. It's optional instead of being there unconditionally so that we can share code for Debug and DebugWithInfcx impls that don't care about whether there is an infcx available but have fields that might care.
There is a trait DebugWithInfcx which allows downstream crates to add impls of the form Debug for OptWithInfcx<...> which would normally be forbidden by orphan rules/coherence.
There is a trait InferCtxtLike to allow us to implement DebugWithInfcx for types that live in rustc_type_ir

This allows debug formatting various ty::* structures with universes shown by using the Debug impl for OptWithInfcx::new(ty, infcx)

This PR does not add DebugWithInfcx impls to absolutely everything that should realistically have them, for example you cannot use OptWithInfcx<Obligation<Predicate>>. I am leaving this to a future PR to do so as it would likely be a lot more work to do.

compiler/rustc_type_ir/src/structural_impls.rs

BoxyUwU · 2023-07-03T14:11:07Z

r? @compiler-errors

bors · 2023-07-05T19:32:16Z

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

compiler-errors

This is fine, though it seems like there's still some more migration to do and more impls to add? Seems like good newcomer work.

Can you make a tracking issue or at least put a checkbox somewhere so that we track the things that still need to be done to make this trait more useful/utilized?

Ideally that one would also have a check-box for "consider ergonomics of adding derive(DebugWithInfcx)" or sth, too. I'd like to see that done eventually or at least a good explanation why it's not a good idea.

compiler-errors · 2023-07-11T18:00:39Z

compiler/rustc_infer/src/infer/mod.rs

+        use InferTy::*;
+        match ty {
+            // FIXME(BoxyUwU): this is kind of jank and means that printing unresolved
+            // ty infers will give you the universe of the var it resolved to not the universe


Wow yeah that's jank. The way regions does it is so much better.

compiler-errors · 2023-07-11T18:00:57Z

compiler/rustc_infer/src/infer/mod.rs

+    }
+
+    fn universe_of_lt(&self, lt: ty::RegionVid) -> Option<ty::UniverseIndex> {
+        Some(self.universe_of_region_vid(lt))


lmao if only we stored the universe not in the eq relation table for ty/ct

Probably perf-negative but I'd like to see it tested, perhaps as a follow-up by someone who's interested in contributing.

compiler-errors · 2023-07-11T18:01:36Z

compiler/rustc_middle/src/ty/list.rs

@@ -119,6 +120,14 @@ impl<T: fmt::Debug> fmt::Debug for List<T> {
        (**self).fmt(f)
    }
 }
+impl<'tcx, T: super::DebugWithInfcx<TyCtxt<'tcx>>> super::DebugWithInfcx<TyCtxt<'tcx>> for List<T> {


I think ideally, if we found some new contributor, it would be cool if we could derive(DebugWithInfcx) or sth. Manual impls are not the worst thing in the world, though..

Yeah I would love to see a derive(DebugWithInfcx)

compiler-errors · 2023-07-11T18:08:05Z

Also this needs documentation on the DebugWithInfcx and InferCtxtLike traits. Can also be done follow-up, since this is internal machinery, but I'd like to see it at least explain the motivation for the traits existing.

@bors r+

bors · 2023-07-11T18:08:07Z

📌 Commit 3fdb443 has been approved by compiler-errors

It is now in the queue for this repository.

BoxyUwU · 2023-07-11T18:18:15Z

Opened #113582

bors · 2023-07-11T20:54:04Z

⌛ Testing commit 3fdb443 with merge 993deaa...

bors · 2023-07-11T22:40:59Z

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

rust-timer · 2023-07-12T05:52:49Z

Finished benchmarking commit (993deaa): 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.5%	[-0.5%, -0.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.5%	[-0.5%, -0.5%]	1

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)	2.3%	[1.0%, 3.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.8%	[-2.8%, -2.8%]	1
All ❌✅ (primary)	2.3%	[1.0%, 3.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)	3.2%	[3.2%, 3.2%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.2%	[3.2%, 3.2%]	1

Binary size

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

Bootstrap: 656.572s -> 656.821s (0.04%)

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 Jun 23, 2023

This comment has been minimized.

Sign in to view

fmease reviewed Jun 24, 2023

View reviewed changes

compiler/rustc_type_ir/src/structural_impls.rs Outdated Show resolved Hide resolved

BoxyUwU force-pushed the debug_with_infcx branch 3 times, most recently from 4376364 to b4eff64 Compare July 3, 2023 13:30

BoxyUwU changed the title ~~[WIP] add a way to debug format ty:: stuff with universes shown~~ Introduce trait DebugWithInfcx to debug format types with universe info Jul 3, 2023

BoxyUwU marked this pull request as ready for review July 3, 2023 14:10

rustbot assigned compiler-errors Jul 3, 2023

BoxyUwU force-pushed the debug_with_infcx branch from 2eb28c2 to 04201dd Compare July 6, 2023 08:54

This comment has been minimized.

Sign in to view

BoxyUwU force-pushed the debug_with_infcx branch from 04201dd to 345a50c Compare July 6, 2023 09:17

Add a new trait to Debug things with an infcx available

3fdb443

BoxyUwU force-pushed the debug_with_infcx branch from 345a50c to 3fdb443 Compare July 6, 2023 10:36

compiler-errors approved these changes Jul 11, 2023

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 Jul 11, 2023

BoxyUwU mentioned this pull request Jul 11, 2023

Tracking Issue for rustc internal debug printing of infer var universes #113582

Closed

3 tasks

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 11, 2023

bors merged commit 993deaa into rust-lang:master Jul 11, 2023

rustbot added this to the 1.73.0 milestone Jul 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `trait DebugWithInfcx` to debug format types with universe info #112984

Introduce `trait DebugWithInfcx` to debug format types with universe info #112984

BoxyUwU commented Jun 23, 2023 •

edited

Loading

This comment has been minimized.

BoxyUwU commented Jul 3, 2023

bors commented Jul 5, 2023

This comment has been minimized.

compiler-errors left a comment

compiler-errors Jul 11, 2023

compiler-errors Jul 11, 2023

compiler-errors Jul 11, 2023

compiler-errors Jul 11, 2023

BoxyUwU Jul 11, 2023

compiler-errors commented Jul 11, 2023

bors commented Jul 11, 2023

BoxyUwU commented Jul 11, 2023

bors commented Jul 11, 2023

bors commented Jul 11, 2023

rust-timer commented Jul 12, 2023

Introduce trait DebugWithInfcx to debug format types with universe info #112984

Introduce trait DebugWithInfcx to debug format types with universe info #112984

Conversation

BoxyUwU commented Jun 23, 2023 • edited Loading

This comment has been minimized.

BoxyUwU commented Jul 3, 2023

bors commented Jul 5, 2023

This comment has been minimized.

compiler-errors left a comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors commented Jul 11, 2023

bors commented Jul 11, 2023

BoxyUwU commented Jul 11, 2023

bors commented Jul 11, 2023

bors commented Jul 11, 2023

rust-timer commented Jul 12, 2023

Overall result: ✅ improvements - no action needed

Introduce `trait DebugWithInfcx` to debug format types with universe info #112984

Introduce `trait DebugWithInfcx` to debug format types with universe info #112984

BoxyUwU commented Jun 23, 2023 •

edited

Loading