Have the constant manager take ownership of constants. (#1866)

* Have the constant manager take ownership of constants.

Right now the owner of an object of type contant that is in the
|const_pool_| of the constant manager is unclear.  The constant
manager does not delete them, there is no other reasonable owner.  This
causes memory leaks.

This change fixes the memory leaks by having the constant manager
take ownership of the constant that is stores in |const_pool_|.  Other
changes include interface changes to make it explicit that the constant
manager takes ownership of the object when a constant is registered
with the constant manager.

Fixes #1865.

source/opt/constants.cpp

@@ -196,19 +196,19 @@ Instruction* ConstantManager::GetDefiningInstruction(
   }
 }
 
-const Constant* ConstantManager::CreateConstant(
+std::unique_ptr<Constant> ConstantManager::CreateConstant(
     const Type* type, const std::vector<uint32_t>& literal_words_or_ids) const {
   if (literal_words_or_ids.size() == 0) {
     // Constant declared with OpConstantNull
-    return new NullConstant(type);
+    return MakeUnique<NullConstant>(type);
   } else if (auto* bt = type->AsBool()) {
     assert(literal_words_or_ids.size() == 1 &&
            "Bool constant should be declared with one operand");
-    return new BoolConstant(bt, literal_words_or_ids.front());
+    return MakeUnique<BoolConstant>(bt, literal_words_or_ids.front());
   } else if (auto* it = type->AsInteger()) {
-    return new IntConstant(it, literal_words_or_ids);
+    return MakeUnique<IntConstant>(it, literal_words_or_ids);
   } else if (auto* ft = type->AsFloat()) {
-    return new FloatConstant(ft, literal_words_or_ids);
+    return MakeUnique<FloatConstant>(ft, literal_words_or_ids);
   } else if (auto* vt = type->AsVector()) {
     auto components = GetConstantsFromIds(literal_words_or_ids);
     if (components.empty()) return nullptr;
@@ -231,19 +231,19 @@ const Constant* ConstantManager::CreateConstant(
                        return false;
                      }))
       return nullptr;
-    return new VectorConstant(vt, components);
+    return MakeUnique<VectorConstant>(vt, components);
   } else if (auto* mt = type->AsMatrix()) {
     auto components = GetConstantsFromIds(literal_words_or_ids);
     if (components.empty()) return nullptr;
-    return new MatrixConstant(mt, components);
+    return MakeUnique<MatrixConstant>(mt, components);
   } else if (auto* st = type->AsStruct()) {
     auto components = GetConstantsFromIds(literal_words_or_ids);
     if (components.empty()) return nullptr;
-    return new StructConstant(st, components);
+    return MakeUnique<StructConstant>(st, components);
   } else if (auto* at = type->AsArray()) {
     auto components = GetConstantsFromIds(literal_words_or_ids);
     if (components.empty()) return nullptr;
-    return new ArrayConstant(at, components);
+    return MakeUnique<ArrayConstant>(at, components);
   } else {
     return nullptr;
   }
@@ -344,7 +344,7 @@ std::unique_ptr<Instruction> ConstantManager::CreateCompositeInstruction(
 const Constant* ConstantManager::GetConstant(
     const Type* type, const std::vector<uint32_t>& literal_words_or_ids) {
   auto cst = CreateConstant(type, literal_words_or_ids);
-  return cst ? RegisterConstant(cst) : nullptr;
+  return cst ? RegisterConstant(std::move(cst)) : nullptr;
 }
 
 std::vector<const analysis::Constant*> Constant::GetVectorComponents(

source/opt/constants.h

@@ -578,8 +578,11 @@ class ConstantManager {
   // Registers a new constant |cst| in the constant pool. If the constant
   // existed already, it returns a pointer to the previously existing Constant
   // in the pool. Otherwise, it returns |cst|.
-  const Constant* RegisterConstant(const Constant* cst) {
-    auto ret = const_pool_.insert(cst);
+  const Constant* RegisterConstant(std::unique_ptr<Constant> cst) {
+    auto ret = const_pool_.insert(cst.get());
+    if (ret.second) {
+      owned_constants_.emplace_back(std::move(cst));
+    }
     return *ret.first;
   }
 
@@ -633,7 +636,7 @@ class ConstantManager {
   // type, either Bool, Integer or Float. If any of the rules above failed, the
   // creation will fail and nullptr will be returned. If the vector is empty,
   // a NullConstant instance will be created with the given type.
-  const Constant* CreateConstant(
+  std::unique_ptr<Constant> CreateConstant(
       const Type* type,
       const std::vector<uint32_t>& literal_words_or_ids) const;
 
@@ -680,6 +683,10 @@ class ConstantManager {
 
   // The constant pool.  All created constants are registered here.
   std::unordered_set<const Constant*, ConstantHash, ConstantEqual> const_pool_;
+
+  // The constant that are owned by the constant manager.  Every constant in
+  // |const_pool_| should be in |owned_constants_| as well.
+  std::vector<std::unique_ptr<Constant>> owned_constants_;
 };
 
 }  // namespace analysis

source/opt/fold_spec_constant_op_and_composite_pass.cpp

@@ -279,11 +279,10 @@ Instruction* FoldSpecConstantOpAndCompositePass::DoVectorShuffle(
            "Literal index out of bound of the concatenated vector");
     selected_components.push_back(concatenated_components[literal]);
   }
-  auto new_vec_const =
-      new analysis::VectorConstant(result_vec_type, selected_components);
+  auto new_vec_const = MakeUnique<analysis::VectorConstant>(
+      result_vec_type, selected_components);
   auto reg_vec_const =
-      context()->get_constant_mgr()->RegisterConstant(new_vec_const);
-  if (reg_vec_const != new_vec_const) delete new_vec_const;
+      context()->get_constant_mgr()->RegisterConstant(std::move(new_vec_const));
   return context()->get_constant_mgr()->BuildInstructionAndAddToModule(
       reg_vec_const, pos);
 }
@@ -368,11 +367,10 @@ Instruction* FoldSpecConstantOpAndCompositePass::DoComponentWiseOperation(
         assert(false && "Failed to create constants with 32-bit word");
       }
     }
-    auto new_vec_const = new analysis::VectorConstant(result_type->AsVector(),
-                                                      result_vector_components);
-    auto reg_vec_const =
-        context()->get_constant_mgr()->RegisterConstant(new_vec_const);
-    if (reg_vec_const != new_vec_const) delete new_vec_const;
+    auto new_vec_const = MakeUnique<analysis::VectorConstant>(
+        result_type->AsVector(), result_vector_components);
+    auto reg_vec_const = context()->get_constant_mgr()->RegisterConstant(
+        std::move(new_vec_const));
     return context()->get_constant_mgr()->BuildInstructionAndAddToModule(
         reg_vec_const, pos);
   } else {