<div dir="ltr">I think this broke the build with libstdc++ 4.7, which this modules bot appears to use:<div><a href="http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules-2/builds/3250/steps/compile/logs/stdio">http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules-2/builds/3250/steps/compile/logs/stdio</a><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 19, 2016 at 10:50 AM, David Majnemer via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: majnemer<br>
Date: Tue Jul 19 12:50:30 2016<br>
New Revision: 276001<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=276001&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=276001&view=rev</a><br>
Log:<br>
[RegionInfo] Some cleanups<br>
<br>
- Use unique_ptr instead of managing a container of new'd pointers.<br>
- Use range based for loops.<br>
<br>
No functional change is intended.<br>
<br>
Modified:<br>
    llvm/trunk/include/llvm/Analysis/RegionInfo.h<br>
    llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h<br>
<br>
Modified: llvm/trunk/include/llvm/Analysis/RegionInfo.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/RegionInfo.h?rev=276001&r1=276000&r2=276001&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/RegionInfo.h?rev=276001&r1=276000&r2=276001&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Analysis/RegionInfo.h (original)<br>
+++ llvm/trunk/include/llvm/Analysis/RegionInfo.h Tue Jul 19 12:50:30 2016<br>
@@ -39,6 +39,7 @@<br>
<br>
 #include "llvm/ADT/DepthFirstIterator.h"<br>
 #include "llvm/ADT/PointerIntPair.h"<br>
+#include "llvm/ADT/iterator_range.h"<br>
 #include "llvm/IR/CFG.h"<br>
 #include "llvm/IR/Dominators.h"<br>
 #include "llvm/IR/PassManager.h"<br>
@@ -278,7 +279,7 @@ class RegionBase : public RegionNodeBase<br>
   // The subregions of this region.<br>
   RegionSet children;<br>
<br>
-  typedef std::map<BlockT *, RegionNodeT *> BBNodeMapT;<br>
+  typedef std::map<BlockT *, std::unique_ptr<RegionNodeT>> BBNodeMapT;<br>
<br>
   // Save the BasicBlock RegionNodes that are element of this Region.<br>
   mutable BBNodeMapT BBNodeMap;<br>
@@ -634,9 +635,15 @@ public:<br>
<br>
   element_iterator element_begin();<br>
   element_iterator element_end();<br>
+  iterator_range<element_iterator> elements() {<br>
+    return make_range(element_begin(), element_end());<br>
+  }<br>
<br>
   const_element_iterator element_begin() const;<br>
   const_element_iterator element_end() const;<br>
+  iterator_range<const_element_iterator> elements() const {<br>
+    return make_range(element_begin(), element_end());<br>
+  }<br>
   //@}<br>
 };<br>
<br>
<br>
Modified: llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h?rev=276001&r1=276000&r2=276001&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h?rev=276001&r1=276000&r2=276001&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h (original)<br>
+++ llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h Tue Jul 19 12:50:30 2016<br>
@@ -38,12 +38,6 @@ RegionBase<Tr>::RegionBase(BlockT *Entry<br>
<br>
 template <class Tr><br>
 RegionBase<Tr>::~RegionBase() {<br>
-  // Free the cached nodes.<br>
-  for (typename BBNodeMapT::iterator it = BBNodeMap.begin(),<br>
-                                     ie = BBNodeMap.end();<br>
-       it != ie; ++it)<br>
-    delete it->second;<br>
-<br>
   // Only clean the cache for this Region. Caches of child Regions will be<br>
   // cleaned when the child Regions are deleted.<br>
   BBNodeMap.clear();<br>
@@ -71,10 +65,9 @@ void RegionBase<Tr>::replaceEntryRecursi<br>
     RegionQueue.pop_back();<br>
<br>
     R->replaceEntry(NewEntry);<br>
-    for (typename RegionT::const_iterator RI = R->begin(), RE = R->end();<br>
-         RI != RE; ++RI) {<br>
-      if ((*RI)->getEntry() == OldEntry)<br>
-        RegionQueue.push_back(RI->get());<br>
+    for (std::unique_ptr<RegionT> &Child : *R) {<br>
+      if (Child->getEntry() == OldEntry)<br>
+        RegionQueue.push_back(Child.get());<br>
     }<br>
   }<br>
 }<br>
@@ -90,10 +83,9 @@ void RegionBase<Tr>::replaceExitRecursiv<br>
     RegionQueue.pop_back();<br>
<br>
     R->replaceExit(NewExit);<br>
-    for (typename RegionT::const_iterator RI = R->begin(), RE = R->end();<br>
-         RI != RE; ++RI) {<br>
-      if ((*RI)->getExit() == OldExit)<br>
-        RegionQueue.push_back(RI->get());<br>
+    for (std::unique_ptr<RegionT> &Child : *R) {<br>
+      if (Child->getExit() == OldExit)<br>
+        RegionQueue.push_back(Child.get());<br>
     }<br>
   }<br>
 }<br>
@@ -160,13 +152,10 @@ typename Tr::LoopT *RegionBase<Tr>::oute<br>
 template <class Tr><br>
 typename RegionBase<Tr>::BlockT *RegionBase<Tr>::getEnteringBlock() const {<br>
   BlockT *entry = getEntry();<br>
-  BlockT *Pred;<br>
   BlockT *enteringBlock = nullptr;<br>
<br>
-  for (PredIterTy PI = InvBlockTraits::child_begin(entry),<br>
-                  PE = InvBlockTraits::child_end(entry);<br>
-       PI != PE; ++PI) {<br>
-    Pred = *PI;<br>
+  for (BlockT *Pred : make_range(InvBlockTraits::child_begin(entry),<br>
+                                 InvBlockTraits::child_end(entry))) {<br>
     if (DT->getNode(Pred) && !contains(Pred)) {<br>
       if (enteringBlock)<br>
         return nullptr;<br>
@@ -181,16 +170,13 @@ typename RegionBase<Tr>::BlockT *RegionB<br>
 template <class Tr><br>
 typename RegionBase<Tr>::BlockT *RegionBase<Tr>::getExitingBlock() const {<br>
   BlockT *exit = getExit();<br>
-  BlockT *Pred;<br>
   BlockT *exitingBlock = nullptr;<br>
<br>
   if (!exit)<br>
     return nullptr;<br>
<br>
-  for (PredIterTy PI = InvBlockTraits::child_begin(exit),<br>
-                  PE = InvBlockTraits::child_end(exit);<br>
-       PI != PE; ++PI) {<br>
-    Pred = *PI;<br>
+  for (BlockT *Pred : make_range(InvBlockTraits::child_begin(exit),<br>
+                                 InvBlockTraits::child_end(exit))) {<br>
     if (contains(Pred)) {<br>
       if (exitingBlock)<br>
         return nullptr;<br>
@@ -239,19 +225,17 @@ void RegionBase<Tr>::verifyBBInRegion(Bl<br>
<br>
   BlockT *entry = getEntry(), *exit = getExit();<br>
<br>
-  for (SuccIterTy SI = BlockTraits::child_begin(BB),<br>
-                  SE = BlockTraits::child_end(BB);<br>
-       SI != SE; ++SI) {<br>
-    if (!contains(*SI) && exit != *SI)<br>
+  for (BlockT *Succ :<br>
+       make_range(BlockTraits::child_begin(BB), BlockTraits::child_end(BB))) {<br>
+    if (!contains(Succ) && exit != Succ)<br>
       llvm_unreachable("Broken region found: edges leaving the region must go "<br>
                        "to the exit node!");<br>
   }<br>
<br>
   if (entry != BB) {<br>
-    for (PredIterTy SI = InvBlockTraits::child_begin(BB),<br>
-                    SE = InvBlockTraits::child_end(BB);<br>
-         SI != SE; ++SI) {<br>
-      if (!contains(*SI))<br>
+    for (BlockT *Pred : make_range(InvBlockTraits::child_begin(BB),<br>
+                                   InvBlockTraits::child_end(BB))) {<br>
+      if (!contains(Pred))<br>
         llvm_unreachable("Broken region found: edges entering the region must "<br>
                          "go to the entry node!");<br>
     }<br>
@@ -266,11 +250,10 @@ void RegionBase<Tr>::verifyWalk(BlockT *<br>
<br>
   verifyBBInRegion(BB);<br>
<br>
-  for (SuccIterTy SI = BlockTraits::child_begin(BB),<br>
-                  SE = BlockTraits::child_end(BB);<br>
-       SI != SE; ++SI) {<br>
-    if (*SI != exit && visited->find(*SI) == visited->end())<br>
-      verifyWalk(*SI, visited);<br>
+  for (BlockT *Succ :<br>
+       make_range(BlockTraits::child_begin(BB), BlockTraits::child_end(BB))) {<br>
+    if (Succ != exit && visited->find(Succ) == visited->end())<br>
+      verifyWalk(Succ, visited);<br>
   }<br>
 }<br>
<br>
@@ -288,9 +271,8 @@ void RegionBase<Tr>::verifyRegion() cons<br>
<br>
 template <class Tr><br>
 void RegionBase<Tr>::verifyRegionNest() const {<br>
-  for (typename RegionT::const_iterator RI = begin(), RE = end(); RI != RE;<br>
-       ++RI)<br>
-    (*RI)->verifyRegionNest();<br>
+  for (const std::unique_ptr<RegionT> &R : *this)<br>
+    R->verifyRegionNest();<br>
<br>
   verifyRegion();<br>
 }<br>
@@ -345,13 +327,14 @@ typename Tr::RegionNodeT *RegionBase<Tr><br>
<br>
   typename BBNodeMapT::const_iterator at = BBNodeMap.find(BB);<br>
<br>
-  if (at != BBNodeMap.end())<br>
-    return at->second;<br>
-<br>
-  auto Deconst = const_cast<RegionBase<Tr> *>(this);<br>
-  RegionNodeT *NewNode = new RegionNodeT(static_cast<RegionT *>(Deconst), BB);<br>
-  BBNodeMap.insert(std::make_pair(BB, NewNode));<br>
-  return NewNode;<br>
+  if (at == BBNodeMap.end()) {<br>
+    auto Deconst = const_cast<RegionBase<Tr> *>(this);<br>
+    at = BBNodeMap<br>
+             .emplace(BB, make_unique<RegionNodeT>(<br>
+                              static_cast<RegionT *>(Deconst), BB))<br>
+             .first;<br>
+  }<br>
+  return at->second.get();<br>
 }<br>
<br>
 template <class Tr><br>
@@ -365,9 +348,9 @@ typename Tr::RegionNodeT *RegionBase<Tr><br>
<br>
 template <class Tr><br>
 void RegionBase<Tr>::transferChildrenTo(RegionT *To) {<br>
-  for (iterator I = begin(), E = end(); I != E; ++I) {<br>
-    (*I)->parent = To;<br>
-    To->children.push_back(std::move(*I));<br>
+  for (std::unique_ptr<RegionT> &R : *this) {<br>
+    R->parent = To;<br>
+    To->children.push_back(std::move(R));<br>
   }<br>
   children.clear();<br>
 }<br>
@@ -375,9 +358,10 @@ void RegionBase<Tr>::transferChildrenTo(<br>
 template <class Tr><br>
 void RegionBase<Tr>::addSubRegion(RegionT *SubRegion, bool moveChildren) {<br>
   assert(!SubRegion->parent && "SubRegion already has a parent!");<br>
-  assert(std::find_if(begin(), end(), [&](const std::unique_ptr<RegionT> &R) {<br>
-           return R.get() == SubRegion;<br>
-         }) == children.end() &&<br>
+  assert(find_if(*this,<br>
+                 [&](const std::unique_ptr<RegionT> &R) {<br>
+                   return R.get() == SubRegion;<br>
+                 }) == children.end() &&<br>
          "Subregion already exists!");<br>
<br>
   SubRegion->parent = static_cast<RegionT *>(this);<br>
@@ -389,9 +373,9 @@ void RegionBase<Tr>::addSubRegion(Region<br>
   assert(SubRegion->children.empty() &&<br>
          "SubRegions that contain children are not supported");<br>
<br>
-  for (element_iterator I = element_begin(), E = element_end(); I != E; ++I) {<br>
-    if (!(*I)->isSubRegion()) {<br>
-      BlockT *BB = (*I)->template getNodeAs<BlockT>();<br>
+  for (RegionNodeT *Element : elements()) {<br>
+    if (!Element->isSubRegion()) {<br>
+      BlockT *BB = Element->template getNodeAs<BlockT>();<br>
<br>
       if (SubRegion->contains(BB))<br>
         RI->setRegionFor(BB, SubRegion);<br>
@@ -399,12 +383,12 @@ void RegionBase<Tr>::addSubRegion(Region<br>
   }<br>
<br>
   std::vector<std::unique_ptr<RegionT>> Keep;<br>
-  for (iterator I = begin(), E = end(); I != E; ++I) {<br>
-    if (SubRegion->contains(I->get()) && I->get() != SubRegion) {<br>
-      (*I)->parent = SubRegion;<br>
-      SubRegion->children.push_back(std::move(*I));<br>
+  for (std::unique_ptr<RegionT> &R : *this) {<br>
+    if (SubRegion->contains(R.get()) && R.get() != SubRegion) {<br>
+      R->parent = SubRegion;<br>
+      SubRegion->children.push_back(std::move(R));<br>
     } else<br>
-      Keep.push_back(std::move(*I));<br>
+      Keep.push_back(std::move(R));<br>
   }<br>
<br>
   children.clear();<br>
@@ -418,9 +402,10 @@ template <class Tr><br>
 typename Tr::RegionT *RegionBase<Tr>::removeSubRegion(RegionT *Child) {<br>
   assert(Child->parent == this && "Child is not a child of this region!");<br>
   Child->parent = nullptr;<br>
-  typename RegionSet::iterator I = std::find_if(<br>
-      children.begin(), children.end(),<br>
-      [&](const std::unique_ptr<RegionT> &R) { return R.get() == Child; });<br>
+  typename RegionSet::iterator I =<br>
+      find_if(children, [&](const std::unique_ptr<RegionT> &R) {<br>
+        return R.get() == Child;<br>
+      });<br>
   assert(I != children.end() && "Region does not exit. Unable to remove.");<br>
   children.erase(children.begin() + (I - begin()));<br>
   return Child;<br>
@@ -446,10 +431,9 @@ typename Tr::RegionT *RegionBase<Tr>::ge<br>
   RegionT *R = RI->getRegionFor(exit);<br>
<br>
   if (R->getEntry() != exit) {<br>
-    for (PredIterTy PI = InvBlockTraits::child_begin(getExit()),<br>
-                    PE = InvBlockTraits::child_end(getExit());<br>
-         PI != PE; ++PI)<br>
-      if (!contains(*PI))<br>
+    for (BlockT *Pred : make_range(InvBlockTraits::child_begin(getExit()),<br>
+                                   InvBlockTraits::child_end(getExit())))<br>
+      if (!contains(Pred))<br>
         return nullptr;<br>
     if (Tr::getNumSuccessors(exit) == 1)<br>
       return new RegionT(getEntry(), *BlockTraits::child_begin(exit), RI, DT);<br>
@@ -459,10 +443,9 @@ typename Tr::RegionT *RegionBase<Tr>::ge<br>
   while (R->getParent() && R->getParent()->getEntry() == exit)<br>
     R = R->getParent();<br>
<br>
-  for (PredIterTy PI = InvBlockTraits::child_begin(getExit()),<br>
-                  PE = InvBlockTraits::child_end(getExit());<br>
-       PI != PE; ++PI) {<br>
-    if (!(contains(*PI) || R->contains(*PI)))<br>
+  for (BlockT *Pred : make_range(InvBlockTraits::child_begin(getExit()),<br>
+                                 InvBlockTraits::child_end(getExit()))) {<br>
+    if (!(contains(Pred) || R->contains(Pred)))<br>
       return nullptr;<br>
   }<br>
<br>
@@ -487,9 +470,8 @@ void RegionBase<Tr>::print(raw_ostream &<br>
       for (const auto *BB : blocks())<br>
         OS << BB->getName() << ", "; // TODO: remove the last ","<br>
     } else if (Style == PrintRN) {<br>
-      for (const_element_iterator I = element_begin(), E = element_end();<br>
-           I != E; ++I) {<br>
-        OS << **I << ", "; // TODO: remove the last ",<br>
+      for (const RegionNodeT *Element : elements()) {<br>
+        OS << *Element << ", "; // TODO: remove the last ",<br>
       }<br>
     }<br>
<br>
@@ -497,8 +479,8 @@ void RegionBase<Tr>::print(raw_ostream &<br>
   }<br>
<br>
   if (print_tree) {<br>
-    for (const_iterator RI = begin(), RE = end(); RI != RE; ++RI)<br>
-      (*RI)->print(OS, print_tree, level + 1, Style);<br>
+    for (const std::unique_ptr<RegionT> &R : *this)<br>
+      R->print(OS, print_tree, level + 1, Style);<br>
   }<br>
<br>
   if (Style != PrintNone)<br>
@@ -514,15 +496,9 @@ void RegionBase<Tr>::dump() const {<br>
<br>
 template <class Tr><br>
 void RegionBase<Tr>::clearNodeCache() {<br>
-  // Free the cached nodes.<br>
-  for (typename BBNodeMapT::iterator I = BBNodeMap.begin(),<br>
-                                     IE = BBNodeMap.end();<br>
-       I != IE; ++I)<br>
-    delete I->second;<br>
-<br>
   BBNodeMap.clear();<br>
-  for (typename RegionT::iterator RI = begin(), RE = end(); RI != RE; ++RI)<br>
-    (*RI)->clearNodeCache();<br>
+  for (std::unique_ptr<RegionT> &R : *this)<br>
+    R->clearNodeCache();<br>
 }<br>
<br>
 //===----------------------------------------------------------------------===//<br>
@@ -541,12 +517,12 @@ RegionInfoBase<Tr>::~RegionInfoBase() {<br>
 template <class Tr><br>
 void RegionInfoBase<Tr>::verifyBBMap(const RegionT *R) const {<br>
   assert(R && "Re must be non-null");<br>
-  for (auto I = R->element_begin(), E = R->element_end(); I != E; ++I) {<br>
-    if (I->isSubRegion()) {<br>
-      const RegionT *SR = I->template getNodeAs<RegionT>();<br>
+  for (const typename Tr::RegionNodeT *Element : R->elements()) {<br>
+    if (Element->isSubRegion()) {<br>
+      const RegionT *SR = Element->template getNodeAs<RegionT>();<br>
       verifyBBMap(SR);<br>
     } else {<br>
-      BlockT *BB = I->template getNodeAs<BlockT>();<br>
+      BlockT *BB = Element->template getNodeAs<BlockT>();<br>
       if (getRegionFor(BB) != R)<br>
         llvm_unreachable("BB map does not match region nesting");<br>
     }<br>
@@ -556,10 +532,8 @@ void RegionInfoBase<Tr>::verifyBBMap(con<br>
 template <class Tr><br>
 bool RegionInfoBase<Tr>::isCommonDomFrontier(BlockT *BB, BlockT *entry,<br>
                                              BlockT *exit) const {<br>
-  for (PredIterTy PI = InvBlockTraits::child_begin(BB),<br>
-                  PE = InvBlockTraits::child_end(BB);<br>
-       PI != PE; ++PI) {<br>
-    BlockT *P = *PI;<br>
+  for (BlockT *P : make_range(InvBlockTraits::child_begin(BB),<br>
+                              InvBlockTraits::child_end(BB))) {<br>
     if (DT->dominates(entry, P) && !DT->dominates(exit, P))<br>
       return false;<br>
   }<br>
@@ -590,20 +564,18 @@ bool RegionInfoBase<Tr>::isRegion(BlockT<br>
   DST *exitSuccs = &DF->find(exit)->second;<br>
<br>
   // Do not allow edges leaving the region.<br>
-  for (typename DST::iterator SI = entrySuccs->begin(), SE = entrySuccs->end();<br>
-       SI != SE; ++SI) {<br>
-    if (*SI == exit || *SI == entry)<br>
+  for (BlockT *Succ : *entrySuccs) {<br>
+    if (Succ == exit || Succ == entry)<br>
       continue;<br>
-    if (exitSuccs->find(*SI) == exitSuccs->end())<br>
+    if (exitSuccs->find(Succ) == exitSuccs->end())<br>
       return false;<br>
-    if (!isCommonDomFrontier(*SI, entry, exit))<br>
+    if (!isCommonDomFrontier(Succ, entry, exit))<br>
       return false;<br>
   }<br>
<br>
   // Do not allow edges pointing into the region.<br>
-  for (typename DST::iterator SI = exitSuccs->begin(), SE = exitSuccs->end();<br>
-       SI != SE; ++SI) {<br>
-    if (DT->properlyDominates(entry, *SI) && *SI != exit)<br>
+  for (BlockT *Succ : *exitSuccs) {<br>
+    if (DT->properlyDominates(entry, Succ) && Succ != exit)<br>
       return false;<br>
   }<br>
<br>
@@ -663,7 +635,7 @@ typename Tr::RegionT *RegionInfoBase<Tr><br>
<br>
   RegionT *region =<br>
       new RegionT(entry, exit, static_cast<RegionInfoT *>(this), DT);<br>
-  BBtoRegion.insert(std::make_pair(entry, region));<br>
+  BBtoRegion.insert({entry, region});<br>
<br>
 #ifdef EXPENSIVE_CHECKS<br>
   region->verifyRegion();<br>
@@ -758,9 +730,8 @@ void RegionInfoBase<Tr>::buildRegionsTre<br>
     BBtoRegion[BB] = region;<br>
   }<br>
<br>
-  for (typename DomTreeNodeT::iterator CI = N->begin(), CE = N->end(); CI != CE;<br>
-       ++CI) {<br>
-    buildRegionsTree(*CI, region);<br>
+  for (DomTreeNodeBase<BlockT> *C : *N) {<br>
+    buildRegionsTree(C, region);<br>
   }<br>
 }<br>
<br>
@@ -850,10 +821,9 @@ RegionInfoBase<Tr>::getMaxRegionExit(Blo<br>
            ExitR->getParent()->getEntry() == Exit)<br>
       ExitR = ExitR->getParent();<br>
<br>
-    for (PredIterTy PI = InvBlockTraits::child_begin(Exit),<br>
-                    PE = InvBlockTraits::child_end(Exit);<br>
-         PI != PE; ++PI) {<br>
-      if (!R->contains(*PI) && !ExitR->contains(*PI))<br>
+    for (BlockT *Pred : make_range(InvBlockTraits::child_begin(Exit),<br>
+                                   InvBlockTraits::child_end(Exit))) {<br>
+      if (!R->contains(Pred) && !ExitR->contains(Pred))<br>
         break;<br>
     }<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>