Hi rsmith,
We could maintain ownership in 'Chain' rather than 'Modules', though that'll
require a little more reordering in removeModules (if it's OK to just move the
Chain loop/remove_if to after the victim loop, then that's easy)
The main uncertainty I have about this change is whether it's safe to move the
Modules.erase from the start of the victim loop (in removeModules) to the end.
Is that OK? Or do some of the operations in the mody of that loop rely on the
entry to have already been removed from 'Modules'?)
http://reviews.llvm.org/D5980
Files:
include/clang/Serialization/ModuleManager.h
lib/Serialization/ModuleManager.cpp
Index: include/clang/Serialization/ModuleManager.h
===================================================================
--- include/clang/Serialization/ModuleManager.h
+++ include/clang/Serialization/ModuleManager.h
@@ -34,7 +34,7 @@
SmallVector<ModuleFile *, 2> Chain;
/// \brief All loaded modules, indexed by name.
- llvm::DenseMap<const FileEntry *, ModuleFile *> Modules;
+ llvm::DenseMap<const FileEntry *, std::unique_ptr<ModuleFile>> Modules;
/// \brief FileManager that handles translating between filenames and
/// FileEntry *.
Index: lib/Serialization/ModuleManager.cpp
===================================================================
--- lib/Serialization/ModuleManager.cpp
+++ lib/Serialization/ModuleManager.cpp
@@ -37,12 +37,11 @@
}
ModuleFile *ModuleManager::lookup(const FileEntry *File) {
- llvm::DenseMap<const FileEntry *, ModuleFile *>::iterator Known
- = Modules.find(File);
+ auto Known = Modules.find(File);
if (Known == Modules.end())
return nullptr;
- return Known->second;
+ return Known->second.get();
}
std::unique_ptr<llvm::MemoryBuffer>
@@ -78,18 +77,18 @@
}
// Check whether we already loaded this module, before
- ModuleFile *&ModuleEntry = Modules[Entry];
+ std::unique_ptr<ModuleFile> &ModuleEntry = Modules[Entry];
bool NewModule = false;
if (!ModuleEntry) {
// Allocate a new module.
- ModuleFile *New = new ModuleFile(Type, Generation);
+ ModuleEntry = llvm::make_unique<ModuleFile>(Type, Generation);
+ ModuleFile *New = ModuleEntry.get();
New->Index = Chain.size();
New->FileName = FileName.str();
New->File = Entry;
New->ImportLoc = ImportLoc;
Chain.push_back(New);
NewModule = true;
- ModuleEntry = New;
New->InputFilesValidationTimestamp = 0;
if (New->Kind == MK_ImplicitModule) {
@@ -145,24 +144,23 @@
// module is *itself* up to date, but has an out-of-date importer.
Modules.erase(Entry);
Chain.pop_back();
- delete New;
return OutOfDate;
}
}
}
if (ImportedBy) {
ModuleEntry->ImportedBy.insert(ImportedBy);
- ImportedBy->Imports.insert(ModuleEntry);
+ ImportedBy->Imports.insert(ModuleEntry.get());
} else {
if (!ModuleEntry->DirectlyImported)
ModuleEntry->ImportLoc = ImportLoc;
ModuleEntry->DirectlyImported = true;
}
- Module = ModuleEntry;
- return NewModule? NewlyLoaded : AlreadyLoaded;
+ Module = ModuleEntry.get();
+ return NewModule ? NewlyLoaded : AlreadyLoaded;
}
void ModuleManager::removeModules(
@@ -184,8 +182,6 @@
// Delete the modules and erase them from the various structures.
for (ModuleIterator victim = first; victim != last; ++victim) {
- Modules.erase((*victim)->File);
-
if (modMap) {
StringRef ModuleName = (*victim)->ModuleName;
if (Module *mod = modMap->findModule(ModuleName)) {
@@ -199,7 +195,7 @@
if (LoadedSuccessfully.count(*victim) == 0)
FileMgr.invalidateCache((*victim)->File);
- delete *victim;
+ Modules.erase((*victim)->File);
}
// Remove the modules from the chain.
@@ -261,8 +257,6 @@
: FileMgr(FileMgr), GlobalIndex(), FirstVisitState(nullptr) {}
ModuleManager::~ModuleManager() {
- for (unsigned i = 0, e = Chain.size(); i != e; ++i)
- delete Chain[e - i - 1];
delete FirstVisitState;
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
