[PATCH] D28225: Implemented color coding and Vertex labels in XRay Graph

[Dean Michael Berris via Phabricator via llvm-commits]

dberris requested changes to this revision.
dberris added inline comments.
This revision now requires changes to proceed.


================
Comment at: tools/llvm-xray/xray-graph.cc:572
   for (const auto &Record : Trace) {
     // Generate graph, FIXME: better error recovery.
+    auto ER2 = GR.accountRecord(Record);
----------------
dberris wrote:
> Do you need the equivalent of a "keep-going" flag here too (similar to what we have in `llvm-xray account`)?
Okay, now I think you need to report errors encountered that we're ignoring in case we do want to keep going. Accumulating the individual errors into a long list seems unnecessary to me.


================
Comment at: tools/llvm-xray/xray-graph.cc:580-586
+    auto ER2 = GR.accountRecord(Record);
+    if (ER2){
+      Err = joinErrors(std::move(ER2), std::move(Err));
+      if(!GraphKeepGoing)
+        return Err;
     }
   }
----------------
This looks like it'd be better as:

```
if (auto E = GR.accountRecord(Record)) {
  ...
}
```


https://reviews.llvm.org/D28225