r202420 - Remove constructors from FileEntry that prevent owning resources
Ben Langmuir
blangmuir at apple.com
Thu Feb 27 11:14:03 PST 2014
Author: benlangmuir
Date: Thu Feb 27 13:14:03 2014
New Revision: 202420
URL: http://llvm.org/viewvc/llvm-project?rev=202420&view=rev
Log:
Remove constructors from FileEntry that prevent owning resources
This cleans up some constructors that would not be safe once FileEntry
owns the storage for its name. These were already suspect, since they
wouldn't work if the FileEntry had an open file descriptor. The only
user for these constructors was in UniqueFileContainer, which wasn't a
very useful abstraction anyway. So it and UniqueDirContainer have been
replaced with std::map<UniqueID, *>.
This change should not affect anything outside the FileManager.
Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/lib/Basic/FileManager.cpp
Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=202420&r1=202419&r2=202420&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Thu Feb 27 13:14:03 2014
@@ -27,6 +27,7 @@
#include "llvm/Support/Allocator.h"
// FIXME: Enhance libsystem to support inode and other fields in stat.
#include <sys/types.h>
+#include <map>
#ifdef _MSC_VER
typedef unsigned short mode_t;
@@ -76,27 +77,15 @@ class FileEntry {
File.reset(0); // rely on destructor to close File
}
+ FileEntry(const FileEntry &) LLVM_DELETED_FUNCTION;
+ void operator=(const FileEntry &) LLVM_DELETED_FUNCTION;
+
public:
- FileEntry(llvm::sys::fs::UniqueID UniqueID, bool IsNamedPipe, bool InPCH)
- : Name(0), UniqueID(UniqueID), IsNamedPipe(IsNamedPipe), InPCH(InPCH),
- IsValid(false)
- {}
- // Add a default constructor for use with llvm::StringMap
FileEntry()
: Name(0), UniqueID(0, 0), IsNamedPipe(false), InPCH(false),
IsValid(false)
{}
- FileEntry(const FileEntry &FE) {
- memcpy(this, &FE, sizeof(FE));
- assert(!File && "Cannot copy a file-owning FileEntry");
- }
-
- void operator=(const FileEntry &FE) {
- memcpy(this, &FE, sizeof(FE));
- assert(!File && "Cannot assign a file-owning FileEntry");
- }
-
const char *getName() const { return Name; }
bool isValid() const { return IsValid; }
off_t getSize() const { return Size; }
@@ -128,14 +117,11 @@ class FileManager : public RefCountedBas
IntrusiveRefCntPtr<vfs::FileSystem> FS;
FileSystemOptions FileSystemOpts;
- class UniqueDirContainer;
- class UniqueFileContainer;
-
/// \brief Cache for existing real directories.
- UniqueDirContainer &UniqueRealDirs;
+ std::map<llvm::sys::fs::UniqueID, DirectoryEntry> UniqueRealDirs;
/// \brief Cache for existing real files.
- UniqueFileContainer &UniqueRealFiles;
+ std::map<llvm::sys::fs::UniqueID, FileEntry> UniqueRealFiles;
/// \brief The virtual directories that we have allocated.
///
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=202420&r1=202419&r2=202420&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Thu Feb 27 13:14:03 2014
@@ -43,41 +43,6 @@ using namespace clang;
/// represent a filename that doesn't exist on the disk.
#define NON_EXISTENT_FILE reinterpret_cast<FileEntry*>((intptr_t)-1)
-
-class FileManager::UniqueDirContainer {
- /// UniqueDirs - Cache from ID's to existing directories/files.
- std::map<llvm::sys::fs::UniqueID, DirectoryEntry> UniqueDirs;
-
-public:
- /// getDirectory - Return an existing DirectoryEntry with the given
- /// ID's if there is already one; otherwise create and return a
- /// default-constructed DirectoryEntry.
- DirectoryEntry &getDirectory(const llvm::sys::fs::UniqueID &UniqueID) {
- return UniqueDirs[UniqueID];
- }
-
- size_t size() const { return UniqueDirs.size(); }
-};
-
-class FileManager::UniqueFileContainer {
- /// UniqueFiles - Cache from ID's to existing directories/files.
- std::set<FileEntry> UniqueFiles;
-
-public:
- /// getFile - Return an existing FileEntry with the given ID's if
- /// there is already one; otherwise create and return a
- /// default-constructed FileEntry.
- FileEntry &getFile(llvm::sys::fs::UniqueID UniqueID, bool IsNamedPipe,
- bool InPCH) {
- return const_cast<FileEntry &>(
- *UniqueFiles.insert(FileEntry(UniqueID, IsNamedPipe, InPCH)).first);
- }
-
- size_t size() const { return UniqueFiles.size(); }
-
- void erase(const FileEntry *Entry) { UniqueFiles.erase(*Entry); }
-};
-
//===----------------------------------------------------------------------===//
// Common logic.
//===----------------------------------------------------------------------===//
@@ -85,8 +50,6 @@ public:
FileManager::FileManager(const FileSystemOptions &FSO,
IntrusiveRefCntPtr<vfs::FileSystem> FS)
: FS(FS), FileSystemOpts(FSO),
- UniqueRealDirs(*new UniqueDirContainer()),
- UniqueRealFiles(*new UniqueFileContainer()),
SeenDirEntries(64), SeenFileEntries(64), NextFileUID(0) {
NumDirLookups = NumFileLookups = 0;
NumDirCacheMisses = NumFileCacheMisses = 0;
@@ -98,8 +61,6 @@ FileManager::FileManager(const FileSyste
}
FileManager::~FileManager() {
- delete &UniqueRealDirs;
- delete &UniqueRealFiles;
for (unsigned i = 0, e = VirtualFileEntries.size(); i != e; ++i)
delete VirtualFileEntries[i];
for (unsigned i = 0, e = VirtualDirectoryEntries.size(); i != e; ++i)
@@ -243,8 +204,7 @@ const DirectoryEntry *FileManager::getDi
// same inode (this occurs on Unix-like systems when one dir is
// symlinked to another, for example) or the same path (on
// Windows).
- DirectoryEntry &UDE =
- UniqueRealDirs.getDirectory(Data.UniqueID);
+ DirectoryEntry &UDE = UniqueRealDirs[Data.UniqueID];
NamedDirEnt.setValue(&UDE);
if (!UDE.getName()) {
@@ -310,8 +270,7 @@ const FileEntry *FileManager::getFile(St
// It exists. See if we have already opened a file with the same inode.
// This occurs when one dir is symlinked to another, for example.
- FileEntry &UFE =
- UniqueRealFiles.getFile(Data.UniqueID, Data.IsNamedPipe, Data.InPCH);
+ FileEntry &UFE = UniqueRealFiles[Data.UniqueID];
NamedFileEnt.setValue(&UFE);
if (UFE.isValid()) { // Already have an entry with this inode, return it.
@@ -322,14 +281,15 @@ const FileEntry *FileManager::getFile(St
return &UFE;
}
- // Otherwise, we don't have this directory yet, add it.
- // FIXME: Change the name to be a char* that points back to the
- // 'SeenFileEntries' key.
+ // Otherwise, we don't have this file yet, add it.
UFE.Name = InterndFileName;
UFE.Size = Data.Size;
UFE.ModTime = Data.ModTime;
UFE.Dir = DirInfo;
UFE.UID = NextFileUID++;
+ UFE.UniqueID = Data.UniqueID;
+ UFE.IsNamedPipe = Data.IsNamedPipe;
+ UFE.InPCH = Data.InPCH;
UFE.File.reset(F);
UFE.IsValid = true;
return &UFE;
@@ -370,7 +330,7 @@ FileManager::getVirtualFile(StringRef Fi
if (getStatValue(InterndFileName, Data, true, 0) == 0) {
Data.Size = Size;
Data.ModTime = ModificationTime;
- UFE = &UniqueRealFiles.getFile(Data.UniqueID, Data.IsNamedPipe, Data.InPCH);
+ UFE = &UniqueRealFiles[Data.UniqueID];
NamedFileEnt.setValue(UFE);
@@ -383,6 +343,10 @@ FileManager::getVirtualFile(StringRef Fi
// If we already have an entry with this inode, return it.
if (UFE->isValid())
return UFE;
+
+ UFE->UniqueID = Data.UniqueID;
+ UFE->IsNamedPipe = Data.IsNamedPipe;
+ UFE->InPCH = Data.InPCH;
}
if (!UFE) {
@@ -509,7 +473,7 @@ void FileManager::invalidateCache(const
// FileEntry invalidation should not block future optimizations in the file
// caches. Possible alternatives are cache truncation (invalidate last N) or
// invalidation of the whole cache.
- UniqueRealFiles.erase(Entry);
+ UniqueRealFiles.erase(Entry->getUniqueID());
}
More information about the cfe-commits
mailing list