[PATCH] D32118: Cleanup some GraphTraits iteration code

[Alexander Kornienko via Phabricator via llvm-commits]

alexfh added inline comments.


================
Comment at: include/llvm/Support/GenericDomTree.h:659
   void splitBlock(NodeT *NewBB) {
     if (this->IsPostDominators)
+      this->Split<Inverse<NodeT *>>(*this, NewBB);
----------------
Will it work without `this`?


================
Comment at: include/llvm/Support/GraphWriter.h:146
     // Loop over the graph, printing it out...
-    for (node_iterator I = GTraits::nodes_begin(G), E = GTraits::nodes_end(G);
-         I != E; ++I)
-      if (!isNodeHidden(*I))
-        writeNode(*I);
+    for (const auto C : nodes<GraphType>(G))
+      if (!isNodeHidden(C))
----------------
Iterator-based loops excuse the use of `I`, `It`, and similar short variable names, since loop variable is repeated multiple times and refers to an iterator anyway. With range-for I'd generally prefer more meaningful loop variable names (`Node`, `Child`,  `Pred`, `Succ`, etc.) as they tend to be involved in less boilerplate code compared to regular for loops and shorter name doesn't buy much in terms of code cleanliness.


https://reviews.llvm.org/D32118