<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 4, 2015 at 10:59 AM, Tobias Grosser <span dir="ltr"><<a href="mailto:tobias@grosser.es" target="_blank">tobias@grosser.es</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: grosser<br>
Date: Thu Jun  4 12:59:54 2015<br>
New Revision: 239061<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D239061-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=U57NEDybEuB_QCQtcEoqCtz-TB7fNsWsWTnWqpa-tU8&s=m3464XVLczksGOvLcses3KGilS_Ox6648DaefZgIaqU&e=" target="_blank">http://llvm.org/viewvc/llvm-project?rev=239061&view=rev</a><br>
Log:<br>
Use owning pointers to avoid memory leaks<br>
<br>
This fixes a memory leak caused by us not freeing the expanded region nodes.<br>
<br>
Modified:<br>
    polly/trunk/lib/Analysis/ScopDetection.cpp<br>
<br>
Modified: polly/trunk/lib/Analysis/ScopDetection.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_polly_trunk_lib_Analysis_ScopDetection.cpp-3Frev-3D239061-26r1-3D239060-26r2-3D239061-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=U57NEDybEuB_QCQtcEoqCtz-TB7fNsWsWTnWqpa-tU8&s=NhMNR9z-DzQRk6KxQPUj7kB6oObzRVc0v1MQXfQuu2o&e=" target="_blank">http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopDetection.cpp?rev=239061&r1=239060&r2=239061&view=diff</a><br>
==============================================================================<br>
--- polly/trunk/lib/Analysis/ScopDetection.cpp (original)<br>
+++ polly/trunk/lib/Analysis/ScopDetection.cpp Thu Jun  4 12:59:54 2015<br>
@@ -722,15 +722,15 @@ bool ScopDetection::isValidLoop(Loop *L,<br>
<br>
 Region *ScopDetection::expandRegion(Region &R) {<br>
   // Initial no valid region was found (greater than R)<br>
-  Region *LastValidRegion = nullptr;<br>
-  Region *ExpandedRegion = R.getExpandedRegion();<br>
+  std::unique_ptr<Region> LastValidRegion;<br>
+  auto ExpandedRegion = std::unique_ptr<Region>(R.getExpandedRegion());<br></blockquote><div><br>Code like this scares me a bit - if R used to own the expanded region, how does it track not owning it anymore?<br><br>(& if you prefer to write "std::unique_ptr<Region> ExpandedRegion(R.getExpandedRegion())" I'm OK with that - I know Herb Sutter espoused the "always use auto" idea, but it's not universally accepted as the way to go)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
   DEBUG(dbgs() << "\tExpanding " << R.getNameStr() << "\n");<br>
<br>
   while (ExpandedRegion) {<br>
     DetectionContext Context(<br>
-        *ExpandedRegion, *AA, NonAffineSubRegionMap[ExpandedRegion],<br>
-        BoxedLoopsMap[ExpandedRegion], false /* verifying */);<br>
+        *ExpandedRegion, *AA, NonAffineSubRegionMap[ExpandedRegion.get()],<br>
+        BoxedLoopsMap[ExpandedRegion.get()], false /* verifying */);<br>
     DEBUG(dbgs() << "\t\tTrying " << ExpandedRegion->getNameStr() << "\n");<br>
     // Only expand when we did not collect errors.<br>
<br>
@@ -742,25 +742,18 @@ Region *ScopDetection::expandRegion(Regi<br>
       if (!allBlocksValid(Context) || Context.Log.hasErrors())<br>
         break;<br>
<br>
-      // Delete unnecessary regions (allocated by getExpandedRegion)<br>
-      if (LastValidRegion)<br>
-        delete LastValidRegion;<br>
-<br>
       // Store this region, because it is the greatest valid (encountered so<br>
       // far).<br>
-      LastValidRegion = ExpandedRegion;<br>
+      LastValidRegion = std::move(ExpandedRegion);<br>
<br>
       // Create and test the next greater region (if any)<br>
-      ExpandedRegion = ExpandedRegion->getExpandedRegion();<br>
+      ExpandedRegion =<br>
+          std::unique_ptr<Region>(LastValidRegion->getExpandedRegion());<br>
<br>
     } else {<br>
       // Create and test the next greater region (if any)<br>
-      Region *TmpRegion = ExpandedRegion->getExpandedRegion();<br>
-<br>
-      // Delete unnecessary regions (allocated by getExpandedRegion)<br>
-      delete ExpandedRegion;<br>
-<br>
-      ExpandedRegion = TmpRegion;<br>
+      ExpandedRegion =<br>
+          std::unique_ptr<Region>(ExpandedRegion->getExpandedRegion());<br>
     }<br>
   }<br>
<br>
@@ -771,7 +764,7 @@ Region *ScopDetection::expandRegion(Regi<br>
       dbgs() << "\tExpanding " << R.getNameStr() << " failed\n";<br>
   });<br>
<br>
-  return LastValidRegion;<br>
+  return LastValidRegion.release();<br></blockquote><div><br>Should this function return unique_ptr to document this ownership transfer?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
 }<br>
 static bool regionWithoutLoops(Region &R, LoopInfo *LI) {<br>
   for (const BasicBlock *BB : R.blocks())<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>