[PATCH] D29005: [XRay] A graph Class for the llvm-xray graph

Dean Michael Berris via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 2 19:04:48 PST 2017


dberris requested changes to this revision.
dberris added a comment.
This revision now requires changes to proceed.

Aside from addressing @dblaikie's comments, please also make sure that the documentation questions raised earlier are addressed too.

Marking this with "Requested Changes" to remove from my "needs review" list.



================
Comment at: include/llvm/XRay/Graph.h:366
+    bool empty() const { return G.Edges.empty(); }
+    EdgeView(GraphT &_G) : G(_G){};
+  };
----------------
Please do not use names that start with _.


================
Comment at: include/llvm/XRay/Graph.h:369
+
+public:
+  void clear() {
----------------
This section s already public, why the repetition here?


================
Comment at: tools/llvm-xray/xray-graph.cc:476
   calculateEdgeStatistics();
+
   calculateVertexStatistics();
----------------
Why the empty line?


https://reviews.llvm.org/D29005





More information about the llvm-commits mailing list