<div dir="ltr">Thanks! Always love to see cleanup like this :)</div><br><div class="gmail_quote"><div dir="ltr">On Sat, Jan 28, 2017 at 2:35 PM Duncan P. N. Exon Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dexonsmith<br class="gmail_msg">
Date: Sat Jan 28 16:24:01 2017<br class="gmail_msg">
New Revision: 293395<br class="gmail_msg">
<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=293395&view=rev" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=293395&view=rev</a><br class="gmail_msg">
Log:<br class="gmail_msg">
Modules: Clarify ownership of ModuleFile instances in ModuleManager, NFC<br class="gmail_msg">
<br class="gmail_msg">
Use std::unique_ptr to clarify the ownership of the ModuleFile instances in<br class="gmail_msg">
ModuleManager.<br class="gmail_msg">
<br class="gmail_msg">
Modified:<br class="gmail_msg">
cfe/trunk/include/clang/Serialization/ModuleManager.h<br class="gmail_msg">
cfe/trunk/lib/Serialization/ModuleManager.cpp<br class="gmail_msg">
<br class="gmail_msg">
Modified: cfe/trunk/include/clang/Serialization/ModuleManager.h<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ModuleManager.h?rev=293395&r1=293394&r2=293395&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ModuleManager.h?rev=293395&r1=293394&r2=293395&view=diff</a><br class="gmail_msg">
==============================================================================<br class="gmail_msg">
--- cfe/trunk/include/clang/Serialization/ModuleManager.h (original)<br class="gmail_msg">
+++ cfe/trunk/include/clang/Serialization/ModuleManager.h Sat Jan 28 16:24:01 2017<br class="gmail_msg">
@@ -33,7 +33,7 @@ namespace serialization {<br class="gmail_msg">
class ModuleManager {<br class="gmail_msg">
/// \brief The chain of AST files, in the order in which we started to load<br class="gmail_msg">
/// them (this order isn't really useful for anything).<br class="gmail_msg">
- SmallVector<ModuleFile *, 2> Chain;<br class="gmail_msg">
+ SmallVector<std::unique_ptr<ModuleFile>, 2> Chain;<br class="gmail_msg">
<br class="gmail_msg">
/// \brief The chain of non-module PCH files. The first entry is the one named<br class="gmail_msg">
/// by the user, the last one is the one that doesn't depend on anything<br class="gmail_msg">
@@ -112,12 +112,14 @@ class ModuleManager {<br class="gmail_msg">
void returnVisitState(VisitState *State);<br class="gmail_msg">
<br class="gmail_msg">
public:<br class="gmail_msg">
- typedef llvm::pointee_iterator<SmallVectorImpl<ModuleFile *>::iterator><br class="gmail_msg">
+ typedef llvm::pointee_iterator<<br class="gmail_msg">
+ SmallVectorImpl<std::unique_ptr<ModuleFile>>::iterator><br class="gmail_msg">
ModuleIterator;<br class="gmail_msg">
- typedef llvm::pointee_iterator<SmallVectorImpl<ModuleFile *>::const_iterator><br class="gmail_msg">
+ typedef llvm::pointee_iterator<<br class="gmail_msg">
+ SmallVectorImpl<std::unique_ptr<ModuleFile>>::const_iterator><br class="gmail_msg">
ModuleConstIterator;<br class="gmail_msg">
typedef llvm::pointee_iterator<<br class="gmail_msg">
- SmallVectorImpl<ModuleFile *>::reverse_iterator><br class="gmail_msg">
+ SmallVectorImpl<std::unique_ptr<ModuleFile>>::reverse_iterator><br class="gmail_msg">
ModuleReverseIterator;<br class="gmail_msg">
typedef std::pair<uint32_t, StringRef> ModuleOffset;<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp<br class="gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=293395&r1=293394&r2=293395&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=293395&r1=293394&r2=293395&view=diff</a><br class="gmail_msg">
==============================================================================<br class="gmail_msg">
--- cfe/trunk/lib/Serialization/ModuleManager.cpp (original)<br class="gmail_msg">
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp Sat Jan 28 16:24:01 2017<br class="gmail_msg">
@@ -96,30 +96,29 @@ ModuleManager::addModule(StringRef FileN<br class="gmail_msg">
<br class="gmail_msg">
// Check whether we already loaded this module, before<br class="gmail_msg">
ModuleFile *ModuleEntry = Modules[Entry];<br class="gmail_msg">
- bool NewModule = false;<br class="gmail_msg">
+ std::unique_ptr<ModuleFile> NewModule;<br class="gmail_msg">
if (!ModuleEntry) {<br class="gmail_msg">
// Allocate a new module.<br class="gmail_msg">
- NewModule = true;<br class="gmail_msg">
- ModuleEntry = new ModuleFile(Type, Generation);<br class="gmail_msg">
- ModuleEntry->Index = Chain.size();<br class="gmail_msg">
- ModuleEntry->FileName = FileName.str();<br class="gmail_msg">
- ModuleEntry->File = Entry;<br class="gmail_msg">
- ModuleEntry->ImportLoc = ImportLoc;<br class="gmail_msg">
- ModuleEntry->InputFilesValidationTimestamp = 0;<br class="gmail_msg">
+ NewModule = llvm::make_unique<ModuleFile>(Type, Generation);<br class="gmail_msg">
+ NewModule->Index = Chain.size();<br class="gmail_msg">
+ NewModule->FileName = FileName.str();<br class="gmail_msg">
+ NewModule->File = Entry;<br class="gmail_msg">
+ NewModule->ImportLoc = ImportLoc;<br class="gmail_msg">
+ NewModule->InputFilesValidationTimestamp = 0;<br class="gmail_msg">
<br class="gmail_msg">
- if (ModuleEntry->Kind == MK_ImplicitModule) {<br class="gmail_msg">
- std::string TimestampFilename = ModuleEntry->getTimestampFilename();<br class="gmail_msg">
+ if (NewModule->Kind == MK_ImplicitModule) {<br class="gmail_msg">
+ std::string TimestampFilename = NewModule->getTimestampFilename();<br class="gmail_msg">
vfs::Status Status;<br class="gmail_msg">
// A cached stat value would be fine as well.<br class="gmail_msg">
if (!FileMgr.getNoncachedStatValue(TimestampFilename, Status))<br class="gmail_msg">
- ModuleEntry->InputFilesValidationTimestamp =<br class="gmail_msg">
+ NewModule->InputFilesValidationTimestamp =<br class="gmail_msg">
llvm::sys::toTimeT(Status.getLastModificationTime());<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
// Load the contents of the module<br class="gmail_msg">
if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {<br class="gmail_msg">
// The buffer was already provided for us.<br class="gmail_msg">
- ModuleEntry->Buffer = std::move(Buffer);<br class="gmail_msg">
+ NewModule->Buffer = std::move(Buffer);<br class="gmail_msg">
} else {<br class="gmail_msg">
// Open the AST file.<br class="gmail_msg">
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buf(<br class="gmail_msg">
@@ -131,28 +130,28 @@ ModuleManager::addModule(StringRef FileN<br class="gmail_msg">
// ModuleManager it must be the same underlying file.<br class="gmail_msg">
// FIXME: Because FileManager::getFile() doesn't guarantee that it will<br class="gmail_msg">
// give us an open file, this may not be 100% reliable.<br class="gmail_msg">
- Buf = FileMgr.getBufferForFile(ModuleEntry->File,<br class="gmail_msg">
+ Buf = FileMgr.getBufferForFile(NewModule->File,<br class="gmail_msg">
/*IsVolatile=*/false,<br class="gmail_msg">
/*ShouldClose=*/false);<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
if (!Buf) {<br class="gmail_msg">
ErrorStr = Buf.getError().message();<br class="gmail_msg">
- delete ModuleEntry;<br class="gmail_msg">
return Missing;<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
- ModuleEntry->Buffer = std::move(*Buf);<br class="gmail_msg">
+ NewModule->Buffer = std::move(*Buf);<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
// Initialize the stream.<br class="gmail_msg">
- ModuleEntry->Data = PCHContainerRdr.ExtractPCH(*ModuleEntry->Buffer);<br class="gmail_msg">
+ NewModule->Data = PCHContainerRdr.ExtractPCH(*NewModule->Buffer);<br class="gmail_msg">
<br class="gmail_msg">
// Read the signature eagerly now so that we can check it.<br class="gmail_msg">
- if (checkSignature(ReadSignature(ModuleEntry->Data), ExpectedSignature, ErrorStr)) {<br class="gmail_msg">
- delete ModuleEntry;<br class="gmail_msg">
+ if (checkSignature(ReadSignature(NewModule->Data), ExpectedSignature, ErrorStr))<br class="gmail_msg">
return OutOfDate;<br class="gmail_msg">
- }<br class="gmail_msg">
+<br class="gmail_msg">
+ // We're keeping this module. Update the map entry.<br class="gmail_msg">
+ ModuleEntry = NewModule.get();<br class="gmail_msg">
} else if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr)) {<br class="gmail_msg">
return OutOfDate;<br class="gmail_msg">
}<br class="gmail_msg">
@@ -175,7 +174,7 @@ ModuleManager::addModule(StringRef FileN<br class="gmail_msg">
assert(!Modules[Entry] && "module loaded twice");<br class="gmail_msg">
Modules[Entry] = ModuleEntry;<br class="gmail_msg">
<br class="gmail_msg">
- Chain.push_back(ModuleEntry);<br class="gmail_msg">
+ Chain.push_back(std::move(NewModule));<br class="gmail_msg">
if (!ModuleEntry->isModule())<br class="gmail_msg">
PCHChain.push_back(ModuleEntry);<br class="gmail_msg">
if (!ImportedBy)<br class="gmail_msg">
@@ -234,11 +233,9 @@ void ModuleManager::removeModules(<br class="gmail_msg">
// new files that will be renamed over the old ones.<br class="gmail_msg">
if (LoadedSuccessfully.count(&*victim) == 0)<br class="gmail_msg">
FileMgr.invalidateCache(victim->File);<br class="gmail_msg">
-<br class="gmail_msg">
- delete &*victim;<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
- // Remove the modules from the chain.<br class="gmail_msg">
+ // Delete the modules.<br class="gmail_msg">
Chain.erase(Chain.begin() + (first - begin()),<br class="gmail_msg">
Chain.begin() + (last - begin()));<br class="gmail_msg">
}<br class="gmail_msg">
@@ -280,11 +277,9 @@ void ModuleManager::setGlobalIndex(Globa<br class="gmail_msg">
<br class="gmail_msg">
// Notify the global module index about all of the modules we've already<br class="gmail_msg">
// loaded.<br class="gmail_msg">
- for (unsigned I = 0, N = Chain.size(); I != N; ++I) {<br class="gmail_msg">
- if (!GlobalIndex->loadedModuleFile(Chain[I])) {<br class="gmail_msg">
- ModulesInCommonWithGlobalIndex.push_back(Chain[I]);<br class="gmail_msg">
- }<br class="gmail_msg">
- }<br class="gmail_msg">
+ for (ModuleFile &M : *this)<br class="gmail_msg">
+ if (!GlobalIndex->loadedModuleFile(&M))<br class="gmail_msg">
+ ModulesInCommonWithGlobalIndex.push_back(&M);<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
void ModuleManager::moduleFileAccepted(ModuleFile *MF) {<br class="gmail_msg">
@@ -299,11 +294,7 @@ ModuleManager::ModuleManager(FileManager<br class="gmail_msg">
: FileMgr(FileMgr), PCHContainerRdr(PCHContainerRdr), GlobalIndex(),<br class="gmail_msg">
FirstVisitState(nullptr) {}<br class="gmail_msg">
<br class="gmail_msg">
-ModuleManager::~ModuleManager() {<br class="gmail_msg">
- for (unsigned i = 0, e = Chain.size(); i != e; ++i)<br class="gmail_msg">
- delete Chain[e - i - 1];<br class="gmail_msg">
- delete FirstVisitState;<br class="gmail_msg">
-}<br class="gmail_msg">
+ModuleManager::~ModuleManager() { delete FirstVisitState; }<br class="gmail_msg">
<br class="gmail_msg">
void ModuleManager::visit(llvm::function_ref<bool(ModuleFile &M)> Visitor,<br class="gmail_msg">
llvm::SmallPtrSetImpl<ModuleFile *> *ModuleFilesHit) {<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
_______________________________________________<br class="gmail_msg">
cfe-commits mailing list<br class="gmail_msg">
<a href="mailto:cfe-commits@lists.llvm.org" class="gmail_msg" target="_blank">cfe-commits@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br class="gmail_msg">
</blockquote></div>