-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add flow-graph visualization (via graphviz) to rustc #14202
Conversation
oh wait, cant merge yet, I'll rebase. |
Add `EntryPat` and `NodePat` variants to ast_map, so that lookups for id 1 in `let S{val: _x /* pat 2 */} /* pat 1 */ = ...` will actually resolve to the pattern `S{ ... }`, rather than "unknown node", in a function like `node_id_to_str`.
Refine lifetimes in signature for graph node/edge iteration methods. Added `pub` `node_id` and `edge_id` methods that correspond to NodeIndex and EdgeIndex `get` methods (note that the inner index is already `pub` in the struct definitions). (I decided that `get()`, used internally, just looks too generic and that client code is clearer with more explicit method names.)
1. Only insert non-dummy nodes into the exit map. 2. Revise handling of `break` and `continue` forms so that they are not treated as if control falls through to the next node (since it does not, it just jumps to the end or start of the loop body). 3. Fixed support for return expression in flow graph construction.
r? anyone |
@nikomatsakis : while i did say "r? anyone", you may be particularly interested in looking at this. |
@@ -515,6 +515,8 @@ pub fn optgroups() -> Vec<getopts::OptGroup> { | |||
optopt("", "opt-level", "Optimize with possible levels 0-3", "LEVEL"), | |||
optopt( "", "out-dir", "Write output to compiler-chosen filename in <dir>", "DIR"), | |||
optflag("", "parse-only", "Parse only; do not compile, assemble, or link"), | |||
optflagopt("", "pretty-node", | |||
"Pretty-print subtree of input, identified by node-id argument", "NODE"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a fairly specific argument to rustc which is taking a very prominent position as a top-level argument. Perhaps this could be transmitted through some other means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be a -Z
argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--pretty flowgraph=<somenodeid>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look into changing to --pretty flowgraph=<somenodeid>
, since I think that will satisfy all parties involved.
(My thinking was that the --pretty-node
flag would eventually be generalized to apply to the rest of the pretty-printer, but adding a top-level flag for that hypothetical generalization was pretty premature.)
This looks great to me, nice set of tests! I'd like to prevent adding another top-level command line argument, but other than that I'd be fine r+'ing this |
// \l, not the line that follows; so, add \l at end of string | ||
// if not already present, ensuring last line gets left-aligned | ||
// as well. | ||
let mut last_two : Vec<_> = s.chars().rev().take(2).collect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can just be s.ends_with("\\l")
?
Passing `--pretty flowgraph=<NODEID>` makes rustc print a control flow graph. In pratice, you will also need to pass the additional option: `-o <FILE>` to emit output to a `.dot` file for graphviz. (You can only print the flow-graph for a particular block in the AST.) ---- An interesting implementation detail is the way the code puts both the node index (`cfg::CFGIndex`) and a reference to the payload (`cfg::CFGNode`) into the single `Node` type that is used for labelling and walking the graph. I had once mistakenly thought that I only wanted the `cfg::CFGNode`, but for labelling, you really want the cfg index too, rather than e.g. trying to use the `ast::NodeId` as the label (which breaks down e.g. due to `ast::DUMMY_NODE_ID`). ---- As a drive-by fix, I had to fix `rustc::middle::cfg::construct` interface to reflect changes that have happened on the master branch while I was getting this integrated into the compiler. (The next commit actually adds tests of the `--pretty flowgraph` functionality, so that should ensure that the `rustc::middle::cfg` code does not go stale again.)
Each test works by rendering the flowgraph for the last identified block we see in expanded pretty-printed output, and comparing it (via `diff`) against a checked in "foo.dot-expected.dot" file. Each test post-processes the output to remove NodeIds ` (id=NUM)` so that the expected output is somewhat stable (or at least independent of how we assign NodeIds) and easier for a human to interpret when looking at the expected output file itself. ---- Test writing style notes: I usually tried to write the tests in a way that would avoid duplicate labels in the output rendered flow graph, when possible. The tests that have string literals "unreachable" in the program text are deliberately written that way to remind the reader that the unreachable nodes in the resulting graph are not an error in the control flow computation, but rather a natural consequence of its construction.
Fantastic! |
Is there any way to see the whole(or even part, like OP says) AST via graphviz output with current rustc ?
EDIT2: found another doc: rust/src/test/ui/mir-dataflow/README.md Line 29 in f45f525
EDIT3: Alright, it works if I add it to the function, like:
|
@xftroxgpx It's probably under |
Thanks @eddyb ! I found the MIR stuff you were referring to here: rust/src/test/ui/mir-dataflow/README.md Line 29 in f45f525
eg.
And it works! But as you said, it's not the AST. Would love to see AST graph, if that exists. EDIT: also found the unpretty ones
this ICEs:
EDIT4: ok this works:
shows
EDIT5: Currently |
@xftroxgpx Do you want an AST CFG, or just an AST? Also, may I ask what for? |
@eddyb Ah, just for learning purposes only. I was hoping that I could see, in a graph-way, how rust sees my vars, loans and assignments, but looks like it's just a that MIR one is what I want, lookslike. (if not a higher level one) |
@xftroxgpx Ah, for that, you can just look at the textual MIR format. Maybe we should do a mixed control-flow and data-flow graph? |
minor: import `Either` from `either` This is a clean-up patch to replace `use itertools::Either` with `use either::Either` for the sake of consistency.
Passing
--pretty=flowgraph
makes rustc print a control flow graph.In practice, you will also need to pass the additional options:
-o <FILE>
to emit output to a.dot
file for graphviz, and--pretty-node=<ID>
to specify the node-id for the block you want to print.(You can only print the flow-graph for a particular block in the AST.)
These changes do not generalize the rest of the pretty-printer to
support the
--pretty-node=<ID>
option for printing subtrees of theAST, but that is obviously a potential future feature that we could
add on. It is orthogonal to flow-graph printing, though.