Subzero. Fixes memory leaks.

src/IceGlobalContext.h

Index: src/IceGlobalContext.h
diff --git a/src/IceGlobalContext.h b/src/IceGlobalContext.h
index ff8ff25ed256b1793a01632c4e32c5e7f95fd207..ae4e2550567b9fcc5d77d001b4ec37b6f31792e2 100644
--- a/src/IceGlobalContext.h
+++ b/src/IceGlobalContext.h
@@ -16,8 +16,11 @@
 #define SUBZERO_SRC_ICEGLOBALCONTEXT_H
 
 #include <array>
+#include <functional>
 #include <mutex>
 #include <thread>
+#include <type_traits>
+#include <vector>
 
 #include "IceDefs.h"
 #include "IceClFlags.h"
@@ -210,8 +213,24 @@ public:
     return getFlags().getDisableIRGeneration();
   }
 
-  // Allocate data of type T using the global allocator.
-  template <typename T> T *allocate() { return getAllocator()->Allocate<T>(); }
+  // Allocate data of type T using the global allocator. We allow entites

[Jim Stichnoth, 2015/06/19 23:09:25]

entities

+  // allocated from this global allocator to be either trivially and

[Jim Stichnoth, 2015/06/19 23:09:25]

either ... or

[John, 2015/06/20 00:10:33]

Done.

+  // non-trivially destructible. We optimize the case when T is trivially
+  // destructible by not registering a destructor. Destructors will be invoked
+  // during GlobalContext destruction in the reverse object creation order.
+  template <typename T>
+  typename std::enable_if<std::is_trivially_destructible<T>::value, T>::type *

[Jim Stichnoth, 2015/06/19 23:09:25]

This is a cool trick, but I feel that the user might unknowingly allocate lots
of objects with nontrivial dtors.

What do you think of calling them something like allocate<>() and
allocateWithDtor<>()?  This forces the user to make a conscious choice.

Since misuse would cause such horrible error messages, maybe use static_assert
instead of enable_if<>?

[John, 2015/06/20 00:10:33]

And that's fine. This allocator, essentially, "replaces" the heap so that we
don't have to keep track of memory management (which is bad in my opinion, but
we already have allocators, so...)
This makes sense in the CfgLocalAllocator as those objects need to be deleted
fast. This global pool should simply run destructors when memory is being
reclaimed. If you don't want you type do have a destructor, you're better off
saying

~Foo() = delete

This way, the compiler will yell if you try to destroy the object (including
implicit stack allocations when the go out of scope, and unique_ptrs.)
It would not. clang++ says "something something std::enable_if<>", and it
highlights enable_if. clang++'s gone a long way since g++ cryptic error
messages. :)

[Jim Stichnoth, 2015/06/21 00:15:48]

OK, I buy that.

+  allocate() {
+    return getAllocator()->Allocate<T>();
+  }
+
+  template <typename T>
+  typename std::enable_if<!std::is_trivially_destructible<T>::value, T>::type *
+  allocate() {
+    T *Ret = getAllocator()->Allocate<T>();
+    getDestructors()->emplace_back([Ret]() { Ret->~T(); });
+    return Ret;
+  }
 
   const Intrinsics &getIntrinsicsInfo() const { return IntrinsicsInfo; }
 
@@ -382,8 +401,9 @@ public:
   // until the queue is empty.
   void emitItems();
 
-  // Uses DataLowering to lower Globals. As a side effect, clears the Globals
-  // array.
+  // Uses DataLowering to lower Globals. Side effects:
+  //  - discards the initializer list for the global variable in Globals.
+  //  - clears the Globals array.
   void lowerGlobals(const IceString &SectionSuffix);
 
   // Lowers the profile information.
@@ -403,12 +423,19 @@ public:
 private:
   // Try to ensure mutexes are allocated on separate cache lines.
 
+  // Destructors collaborate with Allocator
   ICE_CACHELINE_BOUNDARY;
   // Managed by getAllocator()
   GlobalLockType AllocLock;
   ArenaAllocator<> Allocator;
 
   ICE_CACHELINE_BOUNDARY;
+  // Managed by getDestructors()
+  typedef std::vector<std::function<void()>> DestructorArray;
+  GlobalLockType DestructorsLock;
+  DestructorArray Destructors;
+
+  ICE_CACHELINE_BOUNDARY;
   // Managed by getConstantPool()
   GlobalLockType ConstPoolLock;
   std::unique_ptr<ConstantPool> ConstPool;
@@ -456,7 +483,7 @@ private:
   // TODO(jpp): move to EmitterContext.
   VariableDeclarationList Globals;
   // TODO(jpp): move to EmitterContext.
-  std::unique_ptr<VariableDeclaration> ProfileBlockInfoVarDecl;
+  VariableDeclaration *ProfileBlockInfoVarDecl;
 
   LockedPtr<ArenaAllocator<>> getAllocator() {
     return LockedPtr<ArenaAllocator<>>(&Allocator, &AllocLock);
@@ -470,6 +497,9 @@ private:
   LockedPtr<TimerList> getTimers() {
     return LockedPtr<TimerList>(&Timers, &TimerLock);
   }
+  LockedPtr<DestructorArray> getDestructors() {
+    return LockedPtr<DestructorArray>(&Destructors, &DestructorsLock);
+  }
 
   void accumulateGlobals(std::unique_ptr<VariableDeclarationList> Globls) {
     if (Globls != nullptr)