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