Charusso added inline comments.

================
Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:151
+        super(ExplodedGraph, self).__init__()
+        self.nodes = collections.defaultdict(ExplodedNode)
+        self.root_id = None
----------------
NoQ wrote:
> Charusso wrote:
> > `nodes` -> `graph`
> Mm, why would i have a field called "graph" in an object called "graph".
Because we are constructing the graph object and it would be still much better 
than LLVM's fancy `Graph TheGraph;` logic. I am okay with a bunch of nodes, 
just those are the graph itself.


================
Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:191
+                                .replace('\\>', '\\\\>') \
+                                .rstrip(',')
+            logging.debug(raw_json)
----------------
NoQ wrote:
> Charusso wrote:
> > I have removed the trailing `,` from the end yesterday so `rstrip(',')` is 
> > not needed.
> > 
> > It would be cool to name the `result[]`, like: `pred_id = result[1]` and 
> > `succ_id = result[2]` or current/previous ID, or something like that.
> > 
> > I also would put the regexes into a function and you could write:
> > `pred_id, succ_id = parse_edge()` or something more simpler.
> > 
> > What is `result[0]` btw? That confused me a little-bit.
> > `pred_id, succ_id = parse_edge()`
> 
> Not really, i'd also have to return whether the match was successful, which 
> kinda makes it as bad as before.
> 
> > What is `result[0]` btw? That confused me a little-bit.
> 
> That's the entire match. 
> https://docs.python.org/3/library/re.html#match-objects
I see, just I am that newbie. Thanks!


================
Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:237
+            if p.sloc is not None:
+                self._dump('<tr><td align="left" width="0">'
+                           '%s:<b>%s</b>:<b>%s</b>:</td>'
----------------
NoQ wrote:
> Charusso wrote:
> > I think tables are left aligned by default, so you could remove your left 
> > alignments.
> Unfortunately in GraphViz they're centered by default.
Well, that is unfortunate.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62638/new/

https://reviews.llvm.org/D62638



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to