Elaborate on dataflow outputs for region constraints

[amandasystems]

No description provided.

[amandasystems]

I see Zed snuck in some Markdown normalisation too while it was at it. Sorry about that, let me know if that's inexcusable and I will revert those changes.

[lqd]

I will revert those changes

Please do yes, if it's not too hard. It will make review easier.

I'll take a closer look when I have more time, but I noted a couple of things:

• "With -Z dump-mir-graphviz=yes, you will also get Graphviz files for the outlives constraints" is that the case? Did you maybe mean -Zdump-mir=nll?
• I don't think these visualizations are from MIR dataflow, so the new paragraph may need to be moved to somewhere more related to borrowck

[amandasystems]

Please do yes, if it's not too hard. It will make review easier.

Done!

"With -Z dump-mir-graphviz=yes, you will also get Graphviz files for the outlives constraints" is that the case? Did you maybe mean -Zdump-mir=nll?

Huh! Apparently, -Z dump-mir=fn is enough to get those. I assumed it wouldn't drop graphviz files without the Graphviz option, but apparently it does! dump-mir=nll also works and now I'm worried it will dump different graphs.

I don't think these visualizations are from MIR dataflow, so the new paragraph may need to be moved to somewhere more related to borrowck

That's probably true; this should be in the borrowck chapter, maybe? Do you have a suggestion off the top of your head or should I go digging?

[lqd]

Not from the top of my head but I will look for one -- but yeah I agree this should be in the borrowck chapter most likely.

[lqd]

(Sorry for taking so long to get back to this.)

So since these are not dataflow outputs, I agree we should add it to the borrow_check chapter.

To do so, I think we should add a dedicated borrow_check/debugging.md page, and have the "Region constraint graphs and their SCCs" section there. We would mention that these come from -Zdump-mir=nll -- and that will give us the occasion to also describe the NLL dumps there in the future.

We can link to that page from the bottom of src/borrow_check.md instead of src/compiler-debugging.md (but we can leave the unflatten paragraph there, it's a good addition next to the graphviz section).

[lqd]

Suggested change

	The [general instructions on debugging dataflow]() also apply to graphs generated from
	The [general instructions on debugging dataflow](../mir/dataflow.md) also apply to graphs generated from

[lqd]

This line will need to be wrapped as well.

[lqd]

Suggested change

	This is particularly useful for complicated region outlives graphs from
	[MIR Dataflow](mir/dataflow.md).
	This is particularly useful for complicated region outlives graphs from
	[the borrow checker](borrow_check/debugging.md).

[jieyouxu]

@lqd btw, you can also edit @amandasystems's branch directly for trivial things like fixing links. We can also try to get this merged so it doesn't drown from merge conflicts. And it's perfectly fine if it still has inaccuracies/omissions, we can always follow-up, it's just docs after all!

[Kobzol]

Sorry, due to me messing up a git operation, we sadly had to force-push the whole commit history of rustc-dev-guide :( If you'd like to update this pull request, you will have to rebase it in a special way onto the new commit history (the new master):

git fetch origin --all
git checkout <pr-branch>
git rebase --onto origin/master origin/master-old
git push --force-with-lease

More context can be found here.

[jyn514]

@lqd @amandasystems what's the state of this PR? if I rebased it onto main for you, do you think it makes sense to merge it as-is? if not, would you have sometime to work on it in the next few weeks?

[amandasystems]

@lqd @amandasystems what's the state of this PR? if I rebased it onto main for you, do you think it makes sense to merge it as-is? if not, would you have sometime to work on it in the next few weeks?

I looked at what the rebase process would look like and balked a bit. I don't remember what I wrote anymore, but given lqd's minor comments it sounds like it was more or less mergeable.

[amandasystems]

Ok I tried the rebase and it seems mergeable. I'll fix @lqd's nits!

[amandasystems]

Ok I should have fixed the nits too now.

[jyn514]

thank you for your work on this!