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