Simplify destruction of Modules in LLVContextImpl.

This avoids copying the container by simply deleting until empty.

While I'd rather move to a stricter ownership semantic (unique_ptr),
SmallPtrSet can't cope with unique_ptr and the ownership semantics here
are a bit incestuous (Module sort of owns itself, but sort of doesn't
(if the LLVMContext is destroyed before the Module, then it deregisters
itself from the context... )).

Ideally Modules would be given to the context, or possibly an
emplace-like function to construct them there. Modules then shouldn't be
destroyed by LLVM API clients, but by interacting with the owner
(LLVMContext) directly (but even then, passing a Module* to LLVMContext
doesn't provide an easy way to destroy the Module, since the set would
be over unique_ptrs and you'd need a heterogenous lookup function which
SmallPtrSet doesn't have either).

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@206794 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/IR/LLVMContextImpl.cpp b/lib/IR/LLVMContextImpl.cpp
index 506b0e3ac5abad6c7bc19a2c8ae1596260abeb32..30a1ca28c57337e99efe0a44205088a923964e66 100644 (file)
--- a/lib/IR/LLVMContextImpl.cpp
+++ b/lib/IR/LLVMContextImpl.cpp
@@ -119,12 +119,11 @@ struct DropFirst {
 }
 
 LLVMContextImpl::~LLVMContextImpl() {
-  // NOTE: We need to delete the contents of OwnedModules, but we have to
-  // duplicate it into a temporary vector, because the destructor of Module
-  // will try to remove itself from OwnedModules set.  This would cause
-  // iterator invalidation if we iterated on the set directly.
-  std::vector<Module*> Modules(OwnedModules.begin(), OwnedModules.end());
-  DeleteContainerPointers(Modules);
+  // NOTE: We need to delete the contents of OwnedModules, but Module's dtor
+  // will call LLVMContextImpl::removeModule, thus invalidating iterators into
+  // the container. Avoid iterators during this operation:
+  while (!OwnedModules.empty())
+    delete *OwnedModules.begin();
   
   // Free the constants.  This is important to do here to ensure that they are
   // freed before the LeakDetector is torn down.