Index: clang/include/clang/Basic/DirectoryEntry.h =================================================================== --- clang/include/clang/Basic/DirectoryEntry.h +++ clang/include/clang/Basic/DirectoryEntry.h @@ -20,6 +20,11 @@ #include "llvm/Support/ErrorOr.h" namespace clang { +namespace FileMgr { + +template class MapEntryOptionalStorage; + +} // end namespace FileMgr /// Cached information about one directory (either on disk or in /// the virtual file system). @@ -37,20 +42,109 @@ /// as it was accessed by the FileManager's client. class DirectoryEntryRef { public: - const DirectoryEntry &getDirEntry() const { return *Entry->getValue(); } + const DirectoryEntry &getDirEntry() const { return *ME->getValue(); } + + StringRef getName() const { return ME->getKey(); } + + using MapEntry = llvm::StringMapEntry>; - StringRef getName() const { return Entry->getKey(); } + const MapEntry &getMapEntry() const { return *ME; } + + DirectoryEntryRef() = delete; + DirectoryEntryRef(MapEntry &ME) : ME(&ME) {} private: - friend class FileManager; + friend class FileMgr::MapEntryOptionalStorage; + struct optional_none_tag {}; - DirectoryEntryRef( - llvm::StringMapEntry> *Entry) - : Entry(Entry) {} + // Private constructor for use by OptionalStorage. + DirectoryEntryRef(optional_none_tag) : ME(nullptr) {} + bool hasOptionalValue() const { return ME; } - const llvm::StringMapEntry> *Entry; + const MapEntry *ME; }; +namespace FileMgr { + +/// Customized storage for refs derived from map entires in FileManager, using +/// the private optional_none_tag to keep it to the size of a single pointer. +template class MapEntryOptionalStorage { + using optional_none_tag = typename RefTy::optional_none_tag; + RefTy MaybeRef; + +public: + MapEntryOptionalStorage() : MaybeRef(optional_none_tag()) {} + + template + explicit MapEntryOptionalStorage(llvm::optional_detail::in_place_t, + ArgTypes &&...Args) + : MaybeRef(std::forward(Args)...) {} + + void reset() { MaybeRef = optional_none_tag(); } + + bool hasValue() const { return MaybeRef.hasOptionalValue(); } + + RefTy &getValue() LLVM_LVALUE_FUNCTION { + assert(hasValue()); + return MaybeRef; + } + RefTy const &getValue() const LLVM_LVALUE_FUNCTION { + assert(hasValue()); + return MaybeRef; + } +#if LLVM_HAS_RVALUE_REFERENCE_THIS + RefTy &&getValue() && { + assert(hasValue()); + return std::move(MaybeRef); + } +#endif + + template void emplace(Args &&...args) { + MaybeRef = RefTy(std::forward(args)...); + } + + MapEntryOptionalStorage &operator=(RefTy Ref) { + MaybeRef = Ref; + return *this; + } +}; + +} // end namespace FileMgr } // end namespace clang +namespace llvm { +namespace optional_detail { + +/// Customize OptionalStorage to use DirectoryEntryRef and +/// its optional_none_tag to keep it the size of a single pointer. +template <> +class OptionalStorage + : public clang::FileMgr::MapEntryOptionalStorage { + using StorageImpl = + clang::FileMgr::MapEntryOptionalStorage; + +public: + OptionalStorage() = default; + + template + explicit OptionalStorage(in_place_t, ArgTypes &&...Args) + : StorageImpl(in_place_t{}, std::forward(Args)...) {} + + OptionalStorage &operator=(clang::DirectoryEntryRef Ref) { + StorageImpl::operator=(Ref); + return *this; + } +}; + +static_assert(sizeof(Optional) == + sizeof(clang::DirectoryEntryRef), + "Optional must avoid size overhead"); + +static_assert( + std::is_trivially_copyable>::value, + "Optional should be trivially copyable"); + +} // end namespace optional_detail +} // end namespace llvm + #endif // LLVM_CLANG_BASIC_DIRECTORYENTRY_H Index: clang/include/clang/Basic/FileEntry.h =================================================================== --- clang/include/clang/Basic/FileEntry.h +++ clang/include/clang/Basic/FileEntry.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_BASIC_FILEENTRY_H #define LLVM_CLANG_BASIC_FILEENTRY_H +#include "clang/Basic/DirectoryEntry.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/PointerUnion.h" #include "llvm/ADT/StringMap.h" @@ -47,7 +48,6 @@ namespace clang { -class DirectoryEntry; class FileEntry; /// A reference to a \c FileEntry that includes the name of the file as it was @@ -58,6 +58,7 @@ const FileEntry &getFileEntry() const { return *ME->second->V.get(); } + DirectoryEntryRef getDir() const { return *ME->second->Dir; } inline bool isValid() const; inline off_t getSize() const; @@ -104,8 +105,11 @@ /// gcc5.3. Once that's no longer supported, change this back. llvm::PointerUnion V; + /// Directory the file was found in. Set if and only if V is a FileEntry. + Optional Dir; + MapValue() = delete; - MapValue(FileEntry &FE) : V(&FE) {} + MapValue(FileEntry &FE, DirectoryEntryRef Dir) : V(&FE), Dir(Dir) {} MapValue(MapEntry &ME) : V(&ME) {} }; @@ -143,8 +147,7 @@ const clang::FileEntryRef::MapEntry &getMapEntry() const { return *ME; } private: - friend class llvm::optional_detail::OptionalStorage< - FileEntryRef, /*is_trivially_copyable*/ true>; + friend class FileMgr::MapEntryOptionalStorage; struct optional_none_tag {}; // Private constructor for use by OptionalStorage. @@ -167,46 +170,21 @@ /// Customize OptionalStorage to use FileEntryRef and its /// optional_none_tag to keep it the size of a single pointer. -template <> class OptionalStorage { - clang::FileEntryRef MaybeRef; +template <> +class OptionalStorage + : public clang::FileMgr::MapEntryOptionalStorage { + using StorageImpl = + clang::FileMgr::MapEntryOptionalStorage; public: - ~OptionalStorage() = default; - OptionalStorage() : MaybeRef(clang::FileEntryRef::optional_none_tag()) {} - OptionalStorage(OptionalStorage const &Other) = default; - OptionalStorage(OptionalStorage &&Other) = default; - OptionalStorage &operator=(OptionalStorage const &Other) = default; - OptionalStorage &operator=(OptionalStorage &&Other) = default; + OptionalStorage() = default; template explicit OptionalStorage(in_place_t, ArgTypes &&...Args) - : MaybeRef(std::forward(Args)...) {} - - void reset() { MaybeRef = clang::FileEntryRef::optional_none_tag(); } - - bool hasValue() const { return MaybeRef.hasOptionalValue(); } - - clang::FileEntryRef &getValue() LLVM_LVALUE_FUNCTION { - assert(hasValue()); - return MaybeRef; - } - clang::FileEntryRef const &getValue() const LLVM_LVALUE_FUNCTION { - assert(hasValue()); - return MaybeRef; - } -#if LLVM_HAS_RVALUE_REFERENCE_THIS - clang::FileEntryRef &&getValue() && { - assert(hasValue()); - return std::move(MaybeRef); - } -#endif - - template void emplace(Args &&...args) { - MaybeRef = clang::FileEntryRef(std::forward(args)...); - } + : StorageImpl(in_place_t{}, std::forward(Args)...) {} OptionalStorage &operator=(clang::FileEntryRef Ref) { - MaybeRef = Ref; + StorageImpl::operator=(Ref); return *this; } }; Index: clang/lib/Basic/FileManager.cpp =================================================================== --- clang/lib/Basic/FileManager.cpp +++ clang/lib/Basic/FileManager.cpp @@ -69,21 +69,22 @@ /// Retrieve the directory that the given file name resides in. /// Filename can point to either a real file or a virtual file. -static llvm::ErrorOr +static llvm::Expected getDirectoryFromFile(FileManager &FileMgr, StringRef Filename, bool CacheFailure) { if (Filename.empty()) - return std::errc::no_such_file_or_directory; + return llvm::errorCodeToError( + make_error_code(std::errc::no_such_file_or_directory)); if (llvm::sys::path::is_separator(Filename[Filename.size() - 1])) - return std::errc::is_a_directory; + return llvm::errorCodeToError(make_error_code(std::errc::is_a_directory)); StringRef DirName = llvm::sys::path::parent_path(Filename); // Use the current directory if file has no path component. if (DirName.empty()) DirName = "."; - return FileMgr.getDirectory(DirName, CacheFailure); + return FileMgr.getDirectoryRef(DirName, CacheFailure); } /// Add all ancestors of the given path (pointing to either a file or @@ -141,7 +142,7 @@ SeenDirEntries.insert({DirName, std::errc::no_such_file_or_directory}); if (!SeenDirInsertResult.second) { if (SeenDirInsertResult.first->second) - return DirectoryEntryRef(&*SeenDirInsertResult.first); + return DirectoryEntryRef(*SeenDirInsertResult.first); return llvm::errorCodeToError(SeenDirInsertResult.first->second.getError()); } @@ -180,7 +181,7 @@ UDE.Name = InterndDirName; } - return DirectoryEntryRef(&NamedDirEnt); + return DirectoryEntryRef(NamedDirEnt); } llvm::ErrorOr @@ -235,14 +236,15 @@ // without a 'sys' subdir will get a cached failure result. auto DirInfoOrErr = getDirectoryFromFile(*this, Filename, CacheFailure); if (!DirInfoOrErr) { // Directory doesn't exist, file can't exist. + std::error_code Err = errorToErrorCode(DirInfoOrErr.takeError()); if (CacheFailure) - NamedFileEnt->second = DirInfoOrErr.getError(); + NamedFileEnt->second = Err; else SeenFileEntries.erase(Filename); - return llvm::errorCodeToError(DirInfoOrErr.getError()); + return llvm::errorCodeToError(Err); } - const DirectoryEntry *DirInfo = *DirInfoOrErr; + DirectoryEntryRef DirInfo = *DirInfoOrErr; // FIXME: Use the directory info to prune this, before doing the stat syscall. // FIXME: This will reduce the # syscalls. @@ -270,12 +272,13 @@ if (Status.getName() == Filename) { // The name matches. Set the FileEntry. - NamedFileEnt->second = FileEntryRef::MapValue(UFE); + NamedFileEnt->second = FileEntryRef::MapValue(UFE, DirInfo); } else { // Name mismatch. We need a redirect. First grab the actual entry we want // to return. auto &Redirection = - *SeenFileEntries.insert({Status.getName(), FileEntryRef::MapValue(UFE)}) + *SeenFileEntries + .insert({Status.getName(), FileEntryRef::MapValue(UFE, DirInfo)}) .first; assert(Redirection.second->V.is() && "filename redirected to a non-canonical filename?"); @@ -299,8 +302,8 @@ // module's structure when its headers/module map are mapped in the VFS. // We should remove this as soon as we can properly support a file having // multiple names. - if (DirInfo != UFE.Dir && Status.IsVFSMapped) - UFE.Dir = DirInfo; + if (&DirInfo.getDirEntry() != UFE.Dir && Status.IsVFSMapped) + UFE.Dir = &DirInfo.getDirEntry(); // Always update LastRef to the last name by which a file was accessed. // FIXME: Neither this nor always using the first reference is correct; we @@ -318,7 +321,7 @@ UFE.LastRef = ReturnedRef; UFE.Size = Status.getSize(); UFE.ModTime = llvm::sys::toTimeT(Status.getLastModificationTime()); - UFE.Dir = DirInfo; + UFE.Dir = &DirInfo.getDirEntry(); UFE.UID = NextFileUID++; UFE.UniqueID = Status.getUniqueID(); UFE.IsNamedPipe = Status.getType() == llvm::sys::fs::file_type::fifo_file; @@ -363,7 +366,8 @@ // Now that all ancestors of Filename are in the cache, the // following call is guaranteed to find the DirectoryEntry from the // cache. - auto DirInfo = getDirectoryFromFile(*this, Filename, /*CacheFailure=*/true); + auto DirInfo = expectedToOptional( + getDirectoryFromFile(*this, Filename, /*CacheFailure=*/true)); assert(DirInfo && "The directory of a virtual file should already be in the cache."); @@ -378,7 +382,7 @@ Status.getUser(), Status.getGroup(), Size, Status.getType(), Status.getPermissions()); - NamedFileEnt.second = FileEntryRef::MapValue(*UFE); + NamedFileEnt.second = FileEntryRef::MapValue(*UFE, *DirInfo); // If we had already opened this file, close it now so we don't // leak the descriptor. We're not going to use the file @@ -399,13 +403,13 @@ } else { VirtualFileEntries.push_back(std::make_unique()); UFE = VirtualFileEntries.back().get(); - NamedFileEnt.second = FileEntryRef::MapValue(*UFE); + NamedFileEnt.second = FileEntryRef::MapValue(*UFE, *DirInfo); } UFE->LastRef = FileEntryRef(NamedFileEnt); UFE->Size = Size; UFE->ModTime = ModificationTime; - UFE->Dir = *DirInfo; + UFE->Dir = &DirInfo->getDirEntry(); UFE->UID = NextFileUID++; UFE->IsValid = true; UFE->File.reset(); @@ -432,7 +436,7 @@ BypassFileEntries.push_back(std::make_unique()); const FileEntry &VFE = VF.getFileEntry(); FileEntry &BFE = *BypassFileEntries.back(); - Insertion.first->second = FileEntryRef::MapValue(BFE); + Insertion.first->second = FileEntryRef::MapValue(BFE, VF.getDir()); BFE.LastRef = FileEntryRef(*Insertion.first); BFE.Size = Status.getSize(); BFE.Dir = VFE.Dir; Index: clang/unittests/Basic/FileEntryTest.cpp =================================================================== --- clang/unittests/Basic/FileEntryTest.cpp +++ clang/unittests/Basic/FileEntryTest.cpp @@ -15,85 +15,97 @@ namespace { -using MapEntry = FileEntryRef::MapEntry; -using MapValue = FileEntryRef::MapValue; -using MapType = StringMap>; - -FileEntryRef addRef(MapType &M, StringRef Name, FileEntry &E) { - return FileEntryRef(*M.insert({Name, MapValue(E)}).first); -} +using FileMap = StringMap>; +using DirMap = StringMap>; + +struct RefMaps { + FileMap Files; + DirMap Dirs; + + DirectoryEntry D; + DirectoryEntryRef DR; + SmallVector, 5> FEs; + + RefMaps() : DR(*Dirs.insert({"dir", D}).first) {} + + FileEntryRef addFile(StringRef Name) { + FEs.push_back(std::make_unique()); + return FileEntryRef( + *Files.insert({Name, FileEntryRef::MapValue(*FEs.back().get(), DR)}) + .first); + } + FileEntryRef addFileAlias(StringRef Name, FileEntryRef Base) { + return FileEntryRef( + *Files + .insert( + {Name, FileEntryRef::MapValue( + const_cast(Base.getFileEntry()), DR)}) + .first); + } +}; TEST(FileEntryTest, FileEntryRef) { - MapType Refs; - FileEntry E1, E2; - FileEntryRef R1 = addRef(Refs, "1", E1); - FileEntryRef R2 = addRef(Refs, "2", E2); - FileEntryRef R1Also = addRef(Refs, "1-also", E1); + RefMaps Refs; + FileEntryRef R1 = Refs.addFile("1"); + FileEntryRef R2 = Refs.addFile("2"); + FileEntryRef R1Also = Refs.addFileAlias("1-also", R1); EXPECT_EQ("1", R1.getName()); EXPECT_EQ("2", R2.getName()); EXPECT_EQ("1-also", R1Also.getName()); - EXPECT_EQ(&E1, &R1.getFileEntry()); - EXPECT_EQ(&E2, &R2.getFileEntry()); - EXPECT_EQ(&E1, &R1Also.getFileEntry()); + EXPECT_NE(&R1.getFileEntry(), &R2.getFileEntry()); + EXPECT_EQ(&R1.getFileEntry(), &R1Also.getFileEntry()); const FileEntry *CE1 = R1; - EXPECT_EQ(CE1, &E1); + EXPECT_EQ(CE1, &R1.getFileEntry()); } TEST(FileEntryTest, OptionalFileEntryRefDegradesToFileEntryPtr) { - MapType Refs; - FileEntry E1, E2; + RefMaps Refs; OptionalFileEntryRefDegradesToFileEntryPtr M0; - OptionalFileEntryRefDegradesToFileEntryPtr M1 = addRef(Refs, "1", E1); - OptionalFileEntryRefDegradesToFileEntryPtr M2 = addRef(Refs, "2", E2); + OptionalFileEntryRefDegradesToFileEntryPtr M1 = Refs.addFile("1"); + OptionalFileEntryRefDegradesToFileEntryPtr M2 = Refs.addFile("2"); OptionalFileEntryRefDegradesToFileEntryPtr M0Also = None; OptionalFileEntryRefDegradesToFileEntryPtr M1Also = - addRef(Refs, "1-also", E1); + Refs.addFileAlias("1-also", *M1); EXPECT_EQ(M0, M0Also); EXPECT_EQ(StringRef("1"), M1->getName()); EXPECT_EQ(StringRef("2"), M2->getName()); EXPECT_EQ(StringRef("1-also"), M1Also->getName()); - EXPECT_EQ(&E1, &M1->getFileEntry()); - EXPECT_EQ(&E2, &M2->getFileEntry()); - EXPECT_EQ(&E1, &M1Also->getFileEntry()); - const FileEntry *CE1 = M1; - EXPECT_EQ(CE1, &E1); + EXPECT_EQ(CE1, &M1->getFileEntry()); } TEST(FileEntryTest, equals) { - MapType Refs; - FileEntry E1, E2; - FileEntryRef R1 = addRef(Refs, "1", E1); - FileEntryRef R2 = addRef(Refs, "2", E2); - FileEntryRef R1Also = addRef(Refs, "1-also", E1); - - EXPECT_EQ(R1, &E1); - EXPECT_EQ(&E1, R1); + RefMaps Refs; + FileEntryRef R1 = Refs.addFile("1"); + FileEntryRef R2 = Refs.addFile("2"); + FileEntryRef R1Also = Refs.addFileAlias("1-also", R1); + + EXPECT_EQ(R1, &R1.getFileEntry()); + EXPECT_EQ(&R1.getFileEntry(), R1); EXPECT_EQ(R1, R1Also); - EXPECT_NE(R1, &E2); - EXPECT_NE(&E2, R1); + EXPECT_NE(R1, &R2.getFileEntry()); + EXPECT_NE(&R2.getFileEntry(), R1); EXPECT_NE(R1, R2); OptionalFileEntryRefDegradesToFileEntryPtr M0; OptionalFileEntryRefDegradesToFileEntryPtr M1 = R1; - EXPECT_EQ(M1, &E1); - EXPECT_EQ(&E1, M1); - EXPECT_NE(M1, &E2); - EXPECT_NE(&E2, M1); + EXPECT_EQ(M1, &R1.getFileEntry()); + EXPECT_EQ(&R1.getFileEntry(), M1); + EXPECT_NE(M1, &R2.getFileEntry()); + EXPECT_NE(&R2.getFileEntry(), M1); } TEST(FileEntryTest, isSameRef) { - MapType Refs; - FileEntry E1, E2; - FileEntryRef R1 = addRef(Refs, "1", E1); - FileEntryRef R2 = addRef(Refs, "2", E2); - FileEntryRef R1Also = addRef(Refs, "1-also", E1); + RefMaps Refs; + FileEntryRef R1 = Refs.addFile("1"); + FileEntryRef R2 = Refs.addFile("2"); + FileEntryRef R1Also = Refs.addFileAlias("1-also", R1); EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1))); EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1.getMapEntry())));