VS2013: LoopDescriptor LoopContainerType can't contain unique_ptr

The loop descriptor must explicitly manage the storage for contained
Loop objects.

Fixes #1262

2 files changed, 43 insertions, 11 deletions

diff --git a/source/opt/loop_descriptor.cpp b/source/opt/loop_descriptor.cpp
index 6869740c..c5318fdf 100644
--- a/source/opt/loop_descriptor.cpp
+++ b/source/opt/loop_descriptor.cpp
@@ -296,7 +296,11 @@ bool Loop::IsLCSSA() const {
   return true;
 }
 
-LoopDescriptor::LoopDescriptor(const Function* f) { PopulateList(f); }
+LoopDescriptor::LoopDescriptor(const Function* f) : loops_() {
+  PopulateList(f);
+}
+
+LoopDescriptor::~LoopDescriptor() { ClearLoops(); }
 
 void LoopDescriptor::PopulateList(const Function* f) {
   IRContext* context = f->GetParent()->context();
@@ -304,7 +308,7 @@ void LoopDescriptor::PopulateList(const Function* f) {
   opt::DominatorAnalysis* dom_analysis =
       context->GetDominatorAnalysis(f, *context->cfg());
 
-  loops_.clear();
+  ClearLoops();
 
   // Post-order traversal of the dominator tree to find all the OpLoopMerge
   // instructions.
@@ -329,14 +333,14 @@ void LoopDescriptor::PopulateList(const Function* f) {
       BasicBlock* header_bb = context->get_instr_block(merge_inst);
 
       // Add the loop to the list of all the loops in the function.
-      loops_.emplace_back(MakeUnique<Loop>(context, dom_analysis, header_bb,
-                                           continue_bb, merge_bb));
-      Loop* current_loop = loops_.back().get();
+      Loop* current_loop =
+          new Loop(context, dom_analysis, header_bb, continue_bb, merge_bb);
+      loops_.push_back(current_loop);
 
       // We have a bottom-up construction, so if this loop has nested-loops,
       // they are by construction at the tail of the loop list.
       for (auto itr = loops_.rbegin() + 1; itr != loops_.rend(); ++itr) {
-        Loop* previous_loop = itr->get();
+        Loop* previous_loop = *itr;
 
         // If the loop already has a parent, then it has been processed.
         if (previous_loop->HasParent()) continue;
@@ -364,10 +368,17 @@ void LoopDescriptor::PopulateList(const Function* f) {
       }
     }
   }
-  for (std::unique_ptr<Loop>& loop : loops_) {
-    if (!loop->HasParent()) dummy_top_loop_.nested_loops_.push_back(loop.get());
+  for (Loop* loop : loops_) {
+    if (!loop->HasParent()) dummy_top_loop_.nested_loops_.push_back(loop);
   }
 }
 
+void LoopDescriptor::ClearLoops() {
+  for (Loop* loop : loops_) {
+    delete loop;
+  }
+  loops_.clear();
+}
+
 }  // namespace ir
 }  // namespace spvtools
diff --git a/source/opt/loop_descriptor.h b/source/opt/loop_descriptor.h
index 3dfb0c1b..fb53b4c5 100644
--- a/source/opt/loop_descriptor.h
+++ b/source/opt/loop_descriptor.h
@@ -256,6 +256,21 @@ class LoopDescriptor {
   // Creates a loop object for all loops found in |f|.
   explicit LoopDescriptor(const Function* f);
 
+  // Disable copy constructor, to avoid double-free on destruction.
+  LoopDescriptor(const LoopDescriptor&) = delete;
+  // Move constructor.
+  LoopDescriptor(LoopDescriptor&& other) {
+    // We need to take ownership of the Loop objects in the other
+    // LoopDescriptor, to avoid double-free.
+    loops_ = std::move(other.loops_);
+    other.loops_.clear();
+    basic_block_to_loop_ = std::move(other.basic_block_to_loop_);
+    other.basic_block_to_loop_.clear();
+  }
+
+  // Destructor
+  ~LoopDescriptor();
+
   // Returns the number of loops found in the function.
   inline size_t NumLoops() const { return loops_.size(); }
 
@@ -264,7 +279,7 @@ class LoopDescriptor {
   inline Loop& GetLoopByIndex(size_t index) const {
     assert(loops_.size() > index &&
            "Index out of range (larger than loop count)");
-    return *loops_[index].get();
+    return *loops_[index];
   }
 
   // Returns the inner most loop that contains the basic block id |block_id|.
@@ -296,7 +311,9 @@ class LoopDescriptor {
   }
 
  private:
-  using LoopContainerType = std::vector<std::unique_ptr<Loop>>;
+  // TODO(dneto): This should be a vector of unique_ptr.  But VisualStudio 2013
+  // is unable to compile it.
+  using LoopContainerType = std::vector<Loop*>;
 
   // Creates loop descriptors for the function |f|.
   void PopulateList(const Function* f);
@@ -308,7 +325,11 @@ class LoopDescriptor {
     return it != basic_block_to_loop_.end() ? it->second : nullptr;
   }
 
-  // A list of all the loops in the function.
+  // Erase all the loop information.
+  void ClearLoops();
+
+  // A list of all the loops in the function.  This variable owns the Loop
+  // objects.
   LoopContainerType loops_;
   // Dummy root: this "loop" is only there to help iterators creation.
   Loop dummy_top_loop_;