Stream: t-compiler/major changes

Topic: Move graphviz code out of the compiler in… compiler-team#380


triagebot (Oct 28 2020 at 05:53, on Zulip):

A new proposal has been announced: Move graphviz code out of the compiler into external crate #380. It will be announced at the next meeting to try and draw attention to it, but usually MCPs are not discussed during triage meetings. If you think this would benefit from discussion amongst the team, consider proposing a design meeting.

lzutao (Oct 28 2020 at 05:59, on Zulip):

@Vishnunarayan K. I. Can we trust the author to keep maintain the crate ?
Or do you mind transferring crate ownership to a rust-lang member ?

oli (Oct 28 2020 at 07:20, on Zulip):

I have both gotten publish as well as git repo push rights for this crate.

Josh Triplett (Oct 28 2020 at 07:57, on Zulip):

Have you added the rust organization owner as a backup? (cc @Pietro Albini for details.)

Pietro Albini (Oct 28 2020 at 10:16, on Zulip):

add rust-lang-owner to the crate

Vishnunarayan K. I. (Oct 28 2020 at 10:32, on Zulip):

Done.

Wesley Wiser (Oct 28 2020 at 13:38, on Zulip):

Oh, wow, this graph diff is awesome!

https://github.com/vn-ki/gsgdt-rs#diffing-two-graphs

Wesley Wiser (Oct 28 2020 at 13:41, on Zulip):

We have support for rendering in "dark mode". Would we be able to still support that with this crate? I see there's a NodeStyles type but I don't know how configurable it is.

Vishnunarayan K. I. (Oct 28 2020 at 13:42, on Zulip):

Yes, but the red and green (in the diff) isn't really configurable now.

I think I should push down the dark mode to the crate. Right now, rustc code will be handling it.

Wesley Wiser (Oct 28 2020 at 13:44, on Zulip):

Got it. That seems reasonable to me. I don't think rustc needs to be super picky about the colors but having a toggle for light/dark mode would be nice.

triagebot (Oct 28 2020 at 13:44, on Zulip):

@T-compiler: Proposal #380 has been seconded, and will be approved in 10 days if no objections are raised.

pnkfelix (Oct 30 2020 at 04:02, on Zulip):

Hmm, is anything still using the rustc_graphviz crate? If so, should we migrate them to gsgdt-rs ?

pnkfelix (Oct 30 2020 at 04:03, on Zulip):

in any case, it might be good to revise the title of this MCP to point out that it is only (so far) migrating the MIR graphviz usage

pnkfelix (Oct 30 2020 at 04:03, on Zulip):

(if I understand the PR correctly, that is)

pnkfelix (Oct 30 2020 at 04:05, on Zulip):

pnkfelix said:

Hmm, is anything still using the rustc_graphviz crate? If so, should we migrate them to gsgdt-rs ?

(okay, yes, a bunch of stuff is still using rustc_graphviz)

pnkfelix (Oct 30 2020 at 04:05, on Zulip):

I suppose another option would be to migrate rustc_graphviz itself to use gsdt-rs

pnkfelix (Oct 30 2020 at 04:06, on Zulip):

that would certainly be less code-churn in the rest of code base, if it can be done at all. But it would also be pretty awful technical debt, and probably unnecessary tech debt.

pnkfelix (Oct 30 2020 at 04:06, on Zulip):

Anyway big :thumbs_up: from me.

Vishnunarayan K. I. (Oct 30 2020 at 05:55, on Zulip):

pnkfelix: (if I understand the PR correctly, that is)

That is only the initial PR. The future PRs will move dataflow to gsgdt.

I suppose another option would be to migrate rustc_graphviz itself to use gsdt-rs

gsgdt needs the content of the nodes and graph structure to do the diff. it isnt very clear how well that would fit in with the trait based rendering of rustc_graphviz. (EDIT: I had misread. migrating just rustc_graphviz to gsgdt is an interesting idea)

Vishnunarayan K. I. (Oct 30 2020 at 06:37, on Zulip):

I want to point out that MIR to graphviz does not use rustc_graphviz currently. so churn in that is inevitable. (dataflow -> graphviz does)

tmandry (Oct 31 2020 at 02:12, on Zulip):

@Rich Kadel fyi, this might be relevant to some of the debug code you've written lately

triagebot (Nov 11 2020 at 12:26, on Zulip):

This proposal has been accepted: #380.

Last update: May 07 2021 at 06:15UTC