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

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 22 22:29:32 PST 2017


(before looking at the code (going off the list of files changed):
Generally it's preferred to have tests included with the code being
added/changed, where at all possible - is that possible in this case?

*looks at code* Yeah, that's a lot of code to be adding that looks like it
could use a variety of tests. Might even be easier to stage parts of it
separately - an iterator & its unit tests at a time, etc. Not sure.
Whatever seems to work best. Generally the smaller a patch the
exponentially faster the review. (to a point - when the changes are so
small they lack context/motivation, that can make the review take longer
trying to understand "where it's going")

On Sun, Jan 22, 2017 at 10:23 PM Alexis Shaw via Phabricator <
reviews at reviews.llvm.org> wrote:

> varno created this revision.
>
> In preparation for graph comparison and filtering, this is a library for
> representing graphs in LLVM. This will enable easier encapsulation and
> reuse
> of graphs in llvm-xray.
>
> Depends on https://reviews.llvm.org/D28999,
> https://reviews.llvm.org/D28225
>
>
> https://reviews.llvm.org/D29005
>
> Files:
>   include/llvm/XRay/Graph.h
>   tools/llvm-xray/xray-graph.cc
>   tools/llvm-xray/xray-graph.h
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170123/368cb3b2/attachment.html>


More information about the llvm-commits mailing list