Change the sections in the module to use the InstructionList class.

This change will replace a number of the
std::vector<std::unique_ptr<Instruction>> member of the module to
InstructionList.  This is for consistency and to make it easier to
delete instructions that are no longer needed.

source/opt/basic_block.h

@@ -131,7 +131,7 @@ inline BasicBlock::BasicBlock(std::unique_ptr<Instruction> label)
     : function_(nullptr), label_(std::move(label)) {}
 
 inline void BasicBlock::AddInstruction(std::unique_ptr<Instruction> i) {
-  insts_.push_back(i.release());
+  insts_.push_back(std::move(i));
 }
 
 inline void BasicBlock::AddInstructions(BasicBlock* bp) {

source/opt/fold_spec_constant_op_and_composite_pass.cpp

@@ -592,7 +592,7 @@ FoldSpecConstantOpAndCompositePass::BuildInstructionAndAddToModule(
   if (!new_inst) return nullptr;
   auto* new_inst_ptr = new_inst.get();
   *pos = pos->InsertBefore(std::move(new_inst));
-  (*pos)++;
+  ++(*pos);
   def_use_mgr_->AnalyzeInstDefUse(new_inst_ptr);
   return new_inst_ptr;
 }

source/opt/instruction_list.cpp

@@ -17,13 +17,6 @@
 namespace spvtools {
 namespace ir {
 
-InstructionList::~InstructionList() {
-  while (!empty()) {
-    Instruction* inst = &front();
-    inst->RemoveFromList();
-    delete inst;
-  }
-}
 
 InstructionList::iterator InstructionList::iterator::InsertBefore(
     std::vector<std::unique_ptr<Instruction>>&& list) {

source/opt/instruction_list.h

@@ -53,12 +53,12 @@ class InstructionList : public utils::IntrusiveList<Instruction> {
   }
 
   // Destroy this list and any instructions in the list.
-  virtual ~InstructionList();
+  inline virtual ~InstructionList();
 
   class iterator : public utils::IntrusiveList<Instruction>::iterator {
    public:
     iterator(const utils::IntrusiveList<Instruction>::iterator& i)
-        : utils::IntrusiveList<Instruction>::iterator(&*i) {}
+        : utils::IntrusiveList<Instruction>::iterator(i) {}
     iterator(Instruction* i) : utils::IntrusiveList<Instruction>::iterator(i) {}
 
     // DEPRECATED: Please use MoveBefore with an InstructionList instead.
@@ -74,6 +74,16 @@ class InstructionList : public utils::IntrusiveList<Instruction> {
     // will be an iterator pointing to the newly inserted node.  The owner of
     // |*i| becomes |*this|
     iterator InsertBefore(std::unique_ptr<Instruction>&& i);
+
+    // Removes the node from the list, and deletes the storage.  Returns a valid
+    // iterator to the next node.
+    iterator Erase() {
+      iterator_template next_node = *this;
+      ++next_node;
+      node_->RemoveFromList();
+      delete node_;
+      return next_node;
+    }
   };
 
   iterator begin() { return utils::IntrusiveList<Instruction>::begin(); }
@@ -84,8 +94,25 @@ class InstructionList : public utils::IntrusiveList<Instruction> {
   const_iterator end() const {
     return utils::IntrusiveList<Instruction>::end();
   }
+
+  void push_back(std::unique_ptr<Instruction>&& inst) {
+    utils::IntrusiveList<Instruction>::push_back(inst.release());
+  }
+
+  // Same as in the base class, except it will delete the data as well.
+  inline void clear();
 };
 
+InstructionList::~InstructionList() { clear(); }
+
+void InstructionList::clear() {
+  while (!empty()) {
+    Instruction* inst = &front();
+    inst->RemoveFromList();
+    delete inst;
+  }
+}
+
 }  // namespace ir
 }  // namespace spvtools
 

source/opt/iterator.h

@@ -121,9 +121,23 @@ class IteratorRange {
   IteratorType end_;
 };
 
+// Returns a (begin, end) iterator pair for the given iterators.
+// The iterators must belong to the same container.
+template<typename IteratorType>
+inline IteratorRange<IteratorType> make_range(IteratorType& begin, IteratorType& end) {
+  return {begin, end};
+}
+
+// Returns a (begin, end) iterator pair for the given iterators.
+// The iterators must belong to the same container.
+template<typename IteratorType>
+inline IteratorRange<IteratorType> make_range(IteratorType&& begin, IteratorType&& end) {
+  return {begin, end};
+}
+
 // Returns a (begin, end) iterator pair for the given container.
-template <typename ValueType,
-          class IteratorType = UptrVectorIterator<ValueType>>
+template<typename ValueType,
+    class IteratorType = UptrVectorIterator<ValueType>>
 inline IteratorRange<IteratorType> make_range(
     std::vector<std::unique_ptr<ValueType>>& container) {
   return {IteratorType(&container, container.begin()),

source/opt/module.cpp

@@ -24,45 +24,40 @@ namespace spvtools {
 namespace ir {
 
 std::vector<Instruction*> Module::GetTypes() {
-  std::vector<Instruction*> insts;
-  for (uint32_t i = 0; i < types_values_.size(); ++i) {
-    if (IsTypeInst(types_values_[i]->opcode()))
-      insts.push_back(types_values_[i].get());
+  std::vector<Instruction*> type_insts;
+  for (auto& inst : types_values_) {
+    if (IsTypeInst(inst.opcode())) type_insts.push_back(&inst);
   }
-  return insts;
+  return type_insts;
 };
 
 std::vector<const Instruction*> Module::GetTypes() const {
-  std::vector<const Instruction*> insts;
-  for (uint32_t i = 0; i < types_values_.size(); ++i) {
-    if (IsTypeInst(types_values_[i]->opcode()))
-      insts.push_back(types_values_[i].get());
+  std::vector<const Instruction*> type_insts;
+  for (auto& inst : types_values_) {
+    if (IsTypeInst(inst.opcode())) type_insts.push_back(&inst);
   }
-  return insts;
+  return type_insts;
 };
 
 std::vector<Instruction*> Module::GetConstants() {
-  std::vector<Instruction*> insts;
-  for (uint32_t i = 0; i < types_values_.size(); ++i) {
-    if (IsConstantInst(types_values_[i]->opcode()))
-      insts.push_back(types_values_[i].get());
+  std::vector<Instruction*> const_insts;
+  for (auto& inst : types_values_) {
+    if (IsConstantInst(inst.opcode())) const_insts.push_back(&inst);
   }
-  return insts;
+  return const_insts;
 };
 
 std::vector<const Instruction*> Module::GetConstants() const {
-  std::vector<const Instruction*> insts;
-  for (uint32_t i = 0; i < types_values_.size(); ++i) {
-    if (IsConstantInst(types_values_[i]->opcode()))
-      insts.push_back(types_values_[i].get());
+  std::vector<const Instruction*> const_insts;
+  for (auto& inst : types_values_) {
+    if (IsConstantInst(inst.opcode())) const_insts.push_back(&inst);
   }
-  return insts;
+  return const_insts;
 };
 
 uint32_t Module::GetGlobalValue(SpvOp opcode) const {
-  for (uint32_t i = 0; i < types_values_.size(); ++i) {
-    if (types_values_[i]->opcode() == opcode)
-      return types_values_[i]->result_id();
+  for (auto& inst : types_values_) {
+    if (inst.opcode() == opcode) return inst.result_id();
   }
   return 0;
 }
@@ -76,31 +71,31 @@ void Module::AddGlobalValue(SpvOp opcode, uint32_t result_id,
 
 void Module::ForEachInst(const std::function<void(Instruction*)>& f,
                          bool run_on_debug_line_insts) {
-#define DELEGATE(i) i->ForEachInst(f, run_on_debug_line_insts)
+#define DELEGATE(i) i.ForEachInst(f, run_on_debug_line_insts)
   for (auto& i : capabilities_) DELEGATE(i);
   for (auto& i : extensions_) DELEGATE(i);
   for (auto& i : ext_inst_imports_) DELEGATE(i);
-  if (memory_model_) DELEGATE(memory_model_);
+  if (memory_model_) memory_model_->ForEachInst(f, run_on_debug_line_insts);
   for (auto& i : entry_points_) DELEGATE(i);
   for (auto& i : execution_modes_) DELEGATE(i);
   for (auto& i : debugs1_) DELEGATE(i);
   for (auto& i : debugs2_) DELEGATE(i);
   for (auto& i : debugs3_) DELEGATE(i);
   for (auto& i : annotations_) DELEGATE(i);
   for (auto& i : types_values_) DELEGATE(i);
-  for (auto& i : functions_) DELEGATE(i);
+  for (auto& i : functions_) i->ForEachInst(f, run_on_debug_line_insts);
 #undef DELEGATE
 }
 
 void Module::ForEachInst(const std::function<void(const Instruction*)>& f,
                          bool run_on_debug_line_insts) const {
-#define DELEGATE(i)                                      \
-  static_cast<const Instruction*>(i.get())->ForEachInst( \
-      f, run_on_debug_line_insts)
+#define DELEGATE(i) i.ForEachInst(f, run_on_debug_line_insts)
   for (auto& i : capabilities_) DELEGATE(i);
   for (auto& i : extensions_) DELEGATE(i);
   for (auto& i : ext_inst_imports_) DELEGATE(i);
-  if (memory_model_) DELEGATE(memory_model_);
+  if (memory_model_)
+    static_cast<const Instruction*>(memory_model_.get())
+        ->ForEachInst(f, run_on_debug_line_insts);
   for (auto& i : entry_points_) DELEGATE(i);
   for (auto& i : execution_modes_) DELEGATE(i);
   for (auto& i : debugs1_) DELEGATE(i);
@@ -147,7 +142,7 @@ uint32_t Module::ComputeIdBound() const {
 
 bool Module::HasCapability(uint32_t cap) {
   for (auto& ci : capabilities_) {
-    uint32_t tcap = ci->GetSingleWordOperand(0);
+    uint32_t tcap = ci.GetSingleWordOperand(0);
     if (tcap == cap) {
       return true;
     }
@@ -157,9 +152,9 @@ bool Module::HasCapability(uint32_t cap) {
 
 uint32_t Module::GetExtInstImportId(const char* extstr) {
   for (auto& ei : ext_inst_imports_)
-    if (!strcmp(extstr, reinterpret_cast<const char*>(
-        &ei->GetInOperand(0).words[0])))
-      return ei->result_id();
+    if (!strcmp(extstr,
+                reinterpret_cast<const char*>(&(ei.GetInOperand(0).words[0]))))
+      return ei.result_id();
   return 0;
 }