Cleanup some GraphTraits iteration code

[yabash]

Summary

Use children<> and nodes<> in appropriate places to cleanup the code.

Also, as part of the cleanup,
change the signature of DominatorTreeBase's Split.
It is a protected non-virtual member function called only twice,
both from within the class,
and the removed passed argument in both cases is '*this'.
The reason for the existence of that argument seems to be that
back before r43115 Split was a free function,
so an argument to get '*this' was needed - but now that is no longer the case.

[alexfh]

include/llvm/Support/GenericDomTree.h

659 ↗

(On Diff #95401)

Will it work without this?

include/llvm/Support/GraphWriter.h

146 ↗

(On Diff #95401)

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.

[yabash]

Updated according to alexfh's comments.

A lot of "this->" in the code could be removed,
however the specific one you mentioned (and some others) couldn't,
since they are members of a templated base class,
and need to be qualified.

[timshen]

LGTM for GraphTraits-related changes (thought I also see an interface change on Split, which I know nothing about).

[alexfh]

LG with one more comment.

include/llvm/Analysis/LoopInfoImpl.h

89 ↗

(On Diff #95408)

Change to emplace_back?

[yabash]

Thanks, fixed.

I don't have commit rights yet,
can someone please help with committing?

[alexfh]

In D32118#728077, @timshen wrote:

LGTM for GraphTraits-related changes (thought I also see an interface change on Split, which I know nothing about).

I suppose, this change needs at least to be mentioned in the patch description along with a short explanation of why it is a no-op (I hope, it is? ;).

[alexfh]

In D32118#729059, @alexfh wrote:

In D32118#728077, @timshen wrote:

LGTM for GraphTraits-related changes (thought I also see an interface change on Split, which I know nothing about).

I suppose, this change needs at least to be mentioned in the patch description along with a short explanation of why it is a no-op (I hope, it is? ;).

(full disclosure: I also don't know anything about Split, neither about the rest of the code here, and was reviewing mostly for coding style and overall sanity of the change.)

[yabash]

Thank you again for your comments,
I have edited the summary to explain the changes in Split -
I hope it makes sense now :)

[alexfh]

Thank you! I'll let Tim commit this patch, if he has no more concerns.

[timshen]

Yes, taken a closer look, the Split() change is an obvious cleanup. Committed.

Thanks!

llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h

template <class BT> struct BlockEdgesAdder {
   typedef GraphTraits<const BlockT *> Successor;
 
   const BlockFrequencyInfoImpl<BT> &BFI;
   explicit BlockEdgesAdder(const BlockFrequencyInfoImpl<BT> &BFI)
       : BFI(BFI) {}
   void operator()(IrreducibleGraph &G, IrreducibleGraph::IrrNode &Irr,
                   const LoopData *OuterLoop) {
     const BlockT *BB = BFI.RPOT[Irr.Node.Index];
-    for (auto I = Successor::child_begin(BB), E = Successor::child_end(BB);
-         I != E; ++I)
-      G.addEdge(Irr, BFI.getNode(*I), OuterLoop);
+    for (const auto Succ : children<const BlockT *>(BB))
+      G.addEdge(Irr, BFI.getNode(Succ), OuterLoop);
   }
 };
 }
 template <class BT>
 void BlockFrequencyInfoImpl<BT>::computeIrreducibleMass(
     LoopData *OuterLoop, std::list<LoopData>::iterator Insert) {
   DEBUG(dbgs() << "analyze-irreducible-in-";
         if (OuterLoop) dbgs() << "loop: " << getLoopName(*OuterLoop) << "\n";
BlockFrequencyInfoImpl<BT>::propagateMassToSuccessors(LoopData *OuterLoop,
   Distribution Dist;
   if (auto *Loop = Working[Node.Index].getPackagedLoop()) {
     assert(Loop != OuterLoop && "Cannot propagate mass in a packaged loop");
     if (!addLoopSuccessorsToDist(OuterLoop, *Loop, Dist))
       // Irreducible backedge.
       return false;
   } else {
     const BlockT *BB = getBlock(Node);
-    for (auto SI = Successor::child_begin(BB), SE = Successor::child_end(BB);
-         SI != SE; ++SI)
-      if (!addToDist(Dist, OuterLoop, Node, getNode(*SI),
-                     getWeightFromBranchProb(BPI->getEdgeProbability(BB, SI))))
+    for (const auto Succ : children<const BlockT *>(BB))
+      if (!addToDist(Dist, OuterLoop, Node, getNode(Succ),
+                     getWeightFromBranchProb(BPI->getEdgeProbability(BB, Succ))))
         // Irreducible backedge.
         return false;
   }
 
   // Distribute mass to successors, saving exit and backedge data in the
   // loop header.
   distributeMass(Node, OuterLoop, Dist);
   return true;

llvm/trunk/include/llvm/Analysis/DominanceFrontierImpl.h

do {
     const DomTreeNodeT *parentNode = currentW->parentNode;
     assert(currentBB && "Invalid work object. Missing current Basic Block");
     assert(currentNode && "Invalid work object. Missing current Node");
     DomSetType &S = this->Frontiers[currentBB];
 
     // Visit each block only once.
     if (visited.insert(currentBB).second) {
       // Loop over CFG successors to calculate DFlocal[currentNode]
-      for (auto SI = BlockTraits::child_begin(currentBB),
-                SE = BlockTraits::child_end(currentBB);
-           SI != SE; ++SI) {
+      for (const auto Succ : children<BlockT *>(currentBB)) {
         // Does Node immediately dominate this successor?
-        if (DT[*SI]->getIDom() != currentNode)
-          S.insert(*SI);
+        if (DT[Succ]->getIDom() != currentNode)
+          S.insert(Succ);
       }
     }
 
     // At this point, S is DFlocal.  Now we union in DFup's of our children...
     // Loop through and visit the nodes that Node immediately dominates (Node's
     // children in the IDomTree)
     bool visitChild = false;
     for (typename DomTreeNodeT::const_iterator NI = currentNode->begin(),

llvm/trunk/include/llvm/Analysis/LoopInfo.h

public:
   void invalidate() { IsInvalid = true; }
 
   /// Return true if this loop is no longer valid.
   bool isInvalid() { return IsInvalid; }
 
   /// True if terminator in the block can branch to another block that is
   /// outside of the current loop.
   bool isLoopExiting(const BlockT *BB) const {
-    typedef GraphTraits<const BlockT*> BlockTraits;
-    for (typename BlockTraits::ChildIteratorType SI =
-         BlockTraits::child_begin(BB),
-         SE = BlockTraits::child_end(BB); SI != SE; ++SI) {
-      if (!contains(*SI))
+    for (const auto Succ : children<const BlockT*>(BB)) {
+      if (!contains(Succ))
         return true;
     }
     return false;
   }
 
   /// Returns true if \p BB is a loop-latch.
   /// A latch block is a block that contains a branch back to the header.
   /// This function is useful when there are multiple latches in a loop
   /// because \fn getLoopLatch will return nullptr in that case.
   bool isLoopLatch(const BlockT *BB) const {
     assert(contains(BB) && "block does not belong to the loop");
 
     BlockT *Header = getHeader();
     auto PredBegin = GraphTraits<Inverse<BlockT*> >::child_begin(Header);
     auto PredEnd = GraphTraits<Inverse<BlockT*> >::child_end(Header);
     return std::find(PredBegin, PredEnd, BB) != PredEnd;
   }
 
   /// Calculate the number of back edges to the loop header.
   unsigned getNumBackEdges() const {
     unsigned NumBackEdges = 0;
     BlockT *H = getHeader();
 
-    typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
-    for (typename InvBlockTraits::ChildIteratorType I =
-         InvBlockTraits::child_begin(H),
-         E = InvBlockTraits::child_end(H); I != E; ++I)
-      if (contains(*I))
+    for (const auto Pred : children<Inverse<BlockT*> >(H))
+      if (contains(Pred))
         ++NumBackEdges;
 
     return NumBackEdges;
   }
 
   //===--------------------------------------------------------------------===//
   // APIs for simple analysis of the loop.
   //
public:
   /// If there is a single latch block for this loop, return it.
   /// A latch block is a block that contains a branch back to the header.
   BlockT *getLoopLatch() const;
 
   /// Return all loop latch blocks of this loop. A latch block is a block that
   /// contains a branch back to the header.
   void getLoopLatches(SmallVectorImpl<BlockT *> &LoopLatches) const {
     BlockT *H = getHeader();
-    typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
-    for (typename InvBlockTraits::ChildIteratorType I =
-         InvBlockTraits::child_begin(H),
-         E = InvBlockTraits::child_end(H); I != E; ++I)
-      if (contains(*I))
-        LoopLatches.push_back(*I);
+    for (const auto Pred : children<Inverse<BlockT*>>(H))
+      if (contains(Pred))
+        LoopLatches.push_back(Pred);
   }
 
   //===--------------------------------------------------------------------===//
   // APIs for updating loop information after changing the CFG
   //
 
   /// This method is used by other analyses to update loop information.
   /// NewBB is set to be a new member of the current loop.

llvm/trunk/include/llvm/Analysis/LoopInfoImpl.h

 
 /// getExitingBlocks - Return all blocks inside the loop that have successors
 /// outside of the loop.  These are the blocks _inside of the current loop_
 /// which branch out.  The returned list is always unique.
 ///
 template<class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::
 getExitingBlocks(SmallVectorImpl<BlockT *> &ExitingBlocks) const {
-  typedef GraphTraits<BlockT*> BlockTraits;
-  for (block_iterator BI = block_begin(), BE = block_end(); BI != BE; ++BI)
-    for (typename BlockTraits::ChildIteratorType I =
-           BlockTraits::child_begin(*BI), E = BlockTraits::child_end(*BI);
-         I != E; ++I)
-      if (!contains(*I)) {
+  for (const auto BB : blocks())
+    for (const auto Succ : children<BlockT*>(BB))
+      if (!contains(Succ)) {
         // Not in current loop? It must be an exit block.
-        ExitingBlocks.push_back(*BI);
+        ExitingBlocks.push_back(BB);
         break;
       }
 }
 
 /// getExitingBlock - If getExitingBlocks would return exactly one block,
 /// return that block. Otherwise return null.
 template<class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getExitingBlock() const {
   SmallVector<BlockT*, 8> ExitingBlocks;
   getExitingBlocks(ExitingBlocks);
   if (ExitingBlocks.size() == 1)
     return ExitingBlocks[0];
   return nullptr;
 }
 
 /// getExitBlocks - Return all of the successor blocks of this loop.  These
 /// are the blocks _outside of the current loop_ which are branched to.
 ///
 template<class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::
 getExitBlocks(SmallVectorImpl<BlockT*> &ExitBlocks) const {
-  typedef GraphTraits<BlockT*> BlockTraits;
-  for (block_iterator BI = block_begin(), BE = block_end(); BI != BE; ++BI)
-    for (typename BlockTraits::ChildIteratorType I =
-           BlockTraits::child_begin(*BI), E = BlockTraits::child_end(*BI);
-         I != E; ++I)
-      if (!contains(*I))
+  for (const auto BB : blocks())
+    for (const auto Succ : children<BlockT*>(BB))
+      if (!contains(Succ))
         // Not in current loop? It must be an exit block.
-        ExitBlocks.push_back(*I);
+        ExitBlocks.push_back(Succ);
 }
 
 /// getExitBlock - If getExitBlocks would return exactly one block,
 /// return that block. Otherwise return null.
 template<class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getExitBlock() const {
   SmallVector<BlockT*, 8> ExitBlocks;
   getExitBlocks(ExitBlocks);
   if (ExitBlocks.size() == 1)
     return ExitBlocks[0];
   return nullptr;
 }
 
 /// getExitEdges - Return all pairs of (_inside_block_,_outside_block_).
 template<class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::
 getExitEdges(SmallVectorImpl<Edge> &ExitEdges) const {
-  typedef GraphTraits<BlockT*> BlockTraits;
-  for (block_iterator BI = block_begin(), BE = block_end(); BI != BE; ++BI)
-    for (typename BlockTraits::ChildIteratorType I =
-           BlockTraits::child_begin(*BI), E = BlockTraits::child_end(*BI);
-         I != E; ++I)
-      if (!contains(*I))
+  for (const auto BB : blocks())
+    for (const auto Succ : children<BlockT*>(BB))
+      if (!contains(Succ))
         // Not in current loop? It must be an exit block.
-        ExitEdges.push_back(Edge(*BI, *I));
+        ExitEdges.emplace_back(BB, Succ);
 }
 
 /// getLoopPreheader - If there is a preheader for this loop, return it.  A
 /// loop has a preheader if there is only one edge to the header of the loop
 /// from outside of the loop.  If this is the case, the block branching to the
 /// header of the loop is the preheader node.
 ///
 /// This method returns null if there is no preheader for the loop.

 ///
 template<class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getLoopPredecessor() const {
   // Keep track of nodes outside the loop branching to the header...
   BlockT *Out = nullptr;
 
   // Loop over the predecessors of the header node...
   BlockT *Header = getHeader();
-  typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
-  for (typename InvBlockTraits::ChildIteratorType PI =
-         InvBlockTraits::child_begin(Header),
-         PE = InvBlockTraits::child_end(Header); PI != PE; ++PI) {
-    typename InvBlockTraits::NodeRef N = *PI;
-    if (!contains(N)) {     // If the block is not in the loop...
-      if (Out && Out != N)
+  for (const auto Pred : children<Inverse<BlockT*>>(Header)) {
+    if (!contains(Pred)) {     // If the block is not in the loop...
+      if (Out && Out != Pred)
         return nullptr;     // Multiple predecessors outside the loop
-      Out = N;
+      Out = Pred;
     }
   }
 
   // Make sure there is only one exit out of the preheader.
   assert(Out && "Header of loop has no predecessors from outside loop?");
   return Out;
 }
 
 /// getLoopLatch - If there is a single latch block for this loop, return it.
 /// A latch block is a block that contains a branch back to the header.
 template<class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getLoopLatch() const {
   BlockT *Header = getHeader();
-  typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
-  typename InvBlockTraits::ChildIteratorType PI =
-    InvBlockTraits::child_begin(Header);
-  typename InvBlockTraits::ChildIteratorType PE =
-    InvBlockTraits::child_end(Header);
   BlockT *Latch = nullptr;
-  for (; PI != PE; ++PI) {
-    typename InvBlockTraits::NodeRef N = *PI;
-    if (contains(N)) {
+  for (const auto Pred : children<Inverse<BlockT*>>(Header)) {
+    if (contains(Pred)) {
       if (Latch) return nullptr;
-      Latch = N;
+      Latch = Pred;
     }
   }
 
   return Latch;
 }
 
 //===----------------------------------------------------------------------===//
 // APIs for updating loop information after changing the CFG
else {
       Subloop->setParentLoop(L);
       ++NumSubloops;
       NumBlocks += Subloop->getBlocks().capacity();
       PredBB = Subloop->getHeader();
       // Continue traversal along predecessors that are not loop-back edges from
       // within this subloop tree itself. Note that a predecessor may directly
       // reach another subloop that is not yet discovered to be a subloop of
       // this loop, which we must traverse.
-      for (typename InvBlockTraits::ChildIteratorType PI =
-             InvBlockTraits::child_begin(PredBB),
-             PE = InvBlockTraits::child_end(PredBB); PI != PE; ++PI) {
-        if (LI->getLoopFor(*PI) != Subloop)
-          ReverseCFGWorklist.push_back(*PI);
+      for (const auto Pred : children<Inverse<BlockT*>>(PredBB)) {
+        if (LI->getLoopFor(Pred) != Subloop)
+          ReverseCFGWorklist.push_back(Pred);
       }
     }
   }
   L->getSubLoopsVector().reserve(NumSubloops);
   L->reserveBlocks(NumBlocks);
 }
 
 /// Populate all loop data in a stable order during a single forward DFS.
analyze(const DominatorTreeBase<BlockT> &DomTree) {
   // Postorder traversal of the dominator tree.
   const DomTreeNodeBase<BlockT> *DomRoot = DomTree.getRootNode();
   for (auto DomNode : post_order(DomRoot)) {
 
     BlockT *Header = DomNode->getBlock();
     SmallVector<BlockT *, 4> Backedges;
 
     // Check each predecessor of the potential loop header.
-    typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
-    for (typename InvBlockTraits::ChildIteratorType PI =
-           InvBlockTraits::child_begin(Header),
-           PE = InvBlockTraits::child_end(Header); PI != PE; ++PI) {
-
-      BlockT *Backedge = *PI;
-
+    for (const auto Backedge : children<Inverse<BlockT*>>(Header)) {
       // If Header dominates predBB, this is a new loop. Collect the backedges.
       if (DomTree.dominates(Header, Backedge)
           && DomTree.isReachableFromEntry(Backedge)) {
         Backedges.push_back(Backedge);
       }
     }
     // Perform a backward CFG traversal to discover and map blocks in this loop.
     if (!Backedges.empty()) {

llvm/trunk/include/llvm/Support/GenericDomTree.h

void reset() {
     Vertex.clear();
     RootNode = nullptr;
     DFSInfoValid = false;
     SlowQueries = 0;
   }
 
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
-  template <class N, class GraphT>
-  void Split(DominatorTreeBaseByGraphTraits<GraphT> &DT,
-             typename GraphT::NodeRef NewBB) {
+  template <class N>
+  void Split(typename GraphTraits<N>::NodeRef NewBB) {
+    using GraphT = GraphTraits<N>;
+    using NodeRef = typename GraphT::NodeRef;
     assert(std::distance(GraphT::child_begin(NewBB),
                          GraphT::child_end(NewBB)) == 1 &&
            "NewBB should have a single successor!");
-    typename GraphT::NodeRef NewBBSucc = *GraphT::child_begin(NewBB);
+    NodeRef NewBBSucc = *GraphT::child_begin(NewBB);
 
-    std::vector<typename GraphT::NodeRef> PredBlocks;
-    typedef GraphTraits<Inverse<N>> InvTraits;
-    for (typename InvTraits::ChildIteratorType
-             PI = InvTraits::child_begin(NewBB),
-             PE = InvTraits::child_end(NewBB);
-         PI != PE; ++PI)
-      PredBlocks.push_back(*PI);
+    std::vector<NodeRef> PredBlocks;
+    for (const auto Pred : children<Inverse<N>>(NewBB))
+      PredBlocks.push_back(Pred);
 
     assert(!PredBlocks.empty() && "No predblocks?");
 
     bool NewBBDominatesNewBBSucc = true;
-    for (typename InvTraits::ChildIteratorType
-             PI = InvTraits::child_begin(NewBBSucc),
-             E = InvTraits::child_end(NewBBSucc);
-         PI != E; ++PI) {
-      typename InvTraits::NodeRef ND = *PI;
-      if (ND != NewBB && !DT.dominates(NewBBSucc, ND) &&
-          DT.isReachableFromEntry(ND)) {
+    for (const auto Pred : children<Inverse<N>>(NewBBSucc)) {
+      if (Pred != NewBB && !dominates(NewBBSucc, Pred) &&
+          isReachableFromEntry(Pred)) {
         NewBBDominatesNewBBSucc = false;
         break;
       }
     }
 
     // Find NewBB's immediate dominator and create new dominator tree node for
     // NewBB.
     NodeT *NewBBIDom = nullptr;
     unsigned i = 0;
     for (i = 0; i < PredBlocks.size(); ++i)
-      if (DT.isReachableFromEntry(PredBlocks[i])) {
+      if (isReachableFromEntry(PredBlocks[i])) {
         NewBBIDom = PredBlocks[i];
         break;
       }
 
     // It's possible that none of the predecessors of NewBB are reachable;
     // in that case, NewBB itself is unreachable, so nothing needs to be
     // changed.
     if (!NewBBIDom)
       return;
 
     for (i = i + 1; i < PredBlocks.size(); ++i) {
-      if (DT.isReachableFromEntry(PredBlocks[i]))
-        NewBBIDom = DT.findNearestCommonDominator(NewBBIDom, PredBlocks[i]);
+      if (isReachableFromEntry(PredBlocks[i]))
+        NewBBIDom = findNearestCommonDominator(NewBBIDom, PredBlocks[i]);
     }
 
     // Create the new dominator tree node... and set the idom of NewBB.
-    DomTreeNodeBase<NodeT> *NewBBNode = DT.addNewBlock(NewBB, NewBBIDom);
+    DomTreeNodeBase<NodeT> *NewBBNode = addNewBlock(NewBB, NewBBIDom);
 
     // If NewBB strictly dominates other blocks, then it is now the immediate
     // dominator of NewBBSucc.  Update the dominator tree as appropriate.
     if (NewBBDominatesNewBBSucc) {
-      DomTreeNodeBase<NodeT> *NewBBSuccNode = DT.getNode(NewBBSucc);
-      DT.changeImmediateDominator(NewBBSuccNode, NewBBNode);
+      DomTreeNodeBase<NodeT> *NewBBSuccNode = getNode(NewBBSucc);
+      changeImmediateDominator(NewBBSuccNode, NewBBNode);
     }
   }
 
 public:
   explicit DominatorTreeBase(bool isPostDom)
       : DominatorBase<NodeT>(isPostDom) {}
 
   DominatorTreeBase(DominatorTreeBase &&Arg)
public:
   /// compare - Return false if the other dominator tree base matches this
   /// dominator tree base. Otherwise return true.
   bool compare(const DominatorTreeBase &Other) const {
 
     const DomTreeNodeMapType &OtherDomTreeNodes = Other.DomTreeNodes;
     if (DomTreeNodes.size() != OtherDomTreeNodes.size())
       return true;
 
-    for (const auto &DomTreeNode : this->DomTreeNodes) {
+    for (const auto &DomTreeNode : DomTreeNodes) {
       NodeT *BB = DomTreeNode.first;
       typename DomTreeNodeMapType::const_iterator OI =
           OtherDomTreeNodes.find(BB);
       if (OI == OtherDomTreeNodes.end())
         return true;
 
       DomTreeNodeBase<NodeT> &MyNd = *DomTreeNode.second;
       DomTreeNodeBase<NodeT> &OtherNd = *OI->second;
void eraseNode(NodeT *BB) {
 
     DomTreeNodes.erase(BB);
   }
 
   /// splitBlock - BB is split and now it has one successor. Update dominator
   /// tree to reflect this change.
   void splitBlock(NodeT *NewBB) {
     if (this->IsPostDominators)
-      this->Split<Inverse<NodeT *>, GraphTraits<Inverse<NodeT *>>>(*this,
-                                                                   NewBB);
+      Split<Inverse<NodeT *>>(NewBB);
     else
-      this->Split<NodeT *, GraphTraits<NodeT *>>(*this, NewBB);
+      Split<NodeT *>(NewBB);
   }
 
   /// print - Convert to human readable form
   ///
   void print(raw_ostream &o) const {
     o << "=============================--------------------------------\n";
     if (this->isPostDominator())
       o << "Inorder PostDominator Tree: ";
     else
       o << "Inorder Dominator Tree: ";
-    if (!this->DFSInfoValid)
+    if (!DFSInfoValid)
       o << "DFSNumbers invalid: " << SlowQueries << " slow queries.";
     o << "\n";
 
     // The postdom tree can have a null root if there are no returns.
     if (getRootNode())
       PrintDomTree<NodeT>(getRootNode(), o, 1);
   }
 
protected:
   DomTreeNodeBase<NodeT> *getNodeForBlock(NodeT *BB) {
     if (DomTreeNodeBase<NodeT> *Node = getNode(BB))
       return Node;
 
     // Haven't calculated this node yet?  Get or calculate the node for the
     // immediate dominator.
     NodeT *IDom = getIDom(BB);
 
-    assert(IDom || this->DomTreeNodes[nullptr]);
+    assert(IDom || DomTreeNodes[nullptr]);
     DomTreeNodeBase<NodeT> *IDomNode = getNodeForBlock(IDom);
 
     // Add a new tree node for this NodeT, and link it as a child of
     // IDomNode
-    return (this->DomTreeNodes[BB] = IDomNode->addChild(
+    return (DomTreeNodes[BB] = IDomNode->addChild(
                 llvm::make_unique<DomTreeNodeBase<NodeT>>(BB, IDomNode))).get();
   }
 
   NodeT *getIDom(NodeT *BB) const { return IDoms.lookup(BB); }
 
   void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
 
 public:
void updateDFSNumbers() const {
     SlowQueries = 0;
     DFSInfoValid = true;
   }
 
   /// recalculate - compute a dominator tree for the given function
   template <class FT> void recalculate(FT &F) {
     typedef GraphTraits<FT *> TraitsTy;
     reset();
-    this->Vertex.push_back(nullptr);
+    Vertex.push_back(nullptr);
 
     if (!this->IsPostDominators) {
       // Initialize root
       NodeT *entry = TraitsTy::getEntryNode(&F);
       addRoot(entry);
 
       Calculate<FT, NodeT *>(*this, F);
     } else {

llvm/trunk/include/llvm/Support/GraphWriter.h

public:
 
   void writeFooter() {
     // Finish off the graph
     O << "}\n";
   }
 
   void writeNodes() {
     // 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 Node : nodes<GraphType>(G))
+      if (!isNodeHidden(Node))
+        writeNode(Node);
   }
 
   bool isNodeHidden(NodeRef Node) {
     return DTraits.isNodeHidden(Node);
   }
 
   void writeNode(NodeRef Node) {
     std::string NodeAttributes = DTraits.getNodeAttributes(Node, G);