dexonsmith created this revision.
dexonsmith added reviewers: arphaman, JDevlieghere.
Herald added a subscriber: ributzka.
dexonsmith requested review of this revision.
Add `FileEntryRef::getDir`, which returns a `DirectoryEntryRef`. This
includes a few changes:
- Customize `OptionalStorage` so that `Optional<DirectoryEntryRef>` is
pointer-sized (like the change made to `Optional<FileEntryRef>`). Factored out
a common class, `FileMgr::MapEntryOptionalStorage`, to reduce the code
duplication.
- Store an `Optional<DirectoryEntryRef>` in `FileEntryRef::MapValue`. This is
set if and only if `MapValue` has a real `FileEntry`.
- Change `FileManager::getFileRef` and `getVirtualFileRef` to use
`getDirectoryRef` and store it in the `StringMap` for `FileEntryRef`.
https://reviews.llvm.org/D90484
Files:
clang/include/clang/Basic/DirectoryEntry.h
clang/include/clang/Basic/FileEntry.h
clang/lib/Basic/FileManager.cpp
clang/unittests/Basic/FileEntryTest.cpp
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<llvm::ErrorOr<MapValue>>;
-
-FileEntryRef addRef(MapType &M, StringRef Name, FileEntry &E) {
- return FileEntryRef(*M.insert({Name, MapValue(E)}).first);
-}
+using FileMap = StringMap<llvm::ErrorOr<FileEntryRef::MapValue>>;
+using DirMap = StringMap<llvm::ErrorOr<DirectoryEntry &>>;
+
+struct RefMaps {
+ FileMap Files;
+ DirMap Dirs;
+
+ DirectoryEntry D;
+ DirectoryEntryRef DR;
+ SmallVector<std::unique_ptr<FileEntry>, 5> FEs;
+
+ RefMaps() : DR(*Dirs.insert({"dir", D}).first) {}
+
+ FileEntryRef addFile(StringRef Name) {
+ FEs.push_back(std::make_unique<FileEntry>());
+ 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<FileEntry &>(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())));
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<const DirectoryEntry *>
+static llvm::Expected<DirectoryEntryRef>
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<const DirectoryEntry *>
@@ -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<FileEntry *>() &&
"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<FileEntry>());
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<FileEntry>());
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/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<FileEntry *>();
}
+ 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<FileEntry *, const void *> V;
+ /// Directory the file was found in. Set if and only if V is a FileEntry.
+ Optional<DirectoryEntryRef> 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<FileEntryRef>;
struct optional_none_tag {};
// Private constructor for use by OptionalStorage.
@@ -167,8 +170,11 @@
/// Customize OptionalStorage<FileEntryRef> to use FileEntryRef and its
/// optional_none_tag to keep it the size of a single pointer.
-template <> class OptionalStorage<clang::FileEntryRef> {
- clang::FileEntryRef MaybeRef = clang::FileEntryRef::optional_none_tag{};
+template <>
+class OptionalStorage<clang::FileEntryRef>
+ : public clang::FileMgr::MapEntryOptionalStorage<clang::FileEntryRef> {
+ using StorageImpl =
+ clang::FileMgr::MapEntryOptionalStorage<clang::FileEntryRef>;
public:
~OptionalStorage() = default;
@@ -180,36 +186,10 @@
template <class... ArgTypes>
constexpr explicit OptionalStorage(in_place_t, ArgTypes &&...Args)
- : MaybeRef(std::forward<ArgTypes>(Args)...) {}
-
- void reset() noexcept { MaybeRef = clang::FileEntryRef::optional_none_tag(); }
-
- constexpr bool hasValue() const noexcept {
- return MaybeRef.hasOptionalValue();
- }
-
- clang::FileEntryRef &getValue() LLVM_LVALUE_FUNCTION noexcept {
- assert(hasValue());
- return MaybeRef;
- }
- constexpr clang::FileEntryRef const &
- getValue() const LLVM_LVALUE_FUNCTION noexcept {
- assert(hasValue());
- return MaybeRef;
- }
-#if LLVM_HAS_RVALUE_REFERENCE_THIS
- clang::FileEntryRef &&getValue() &&noexcept {
- assert(hasValue());
- return std::move(MaybeRef);
- }
-#endif
-
- template <class... Args> void emplace(Args &&...args) {
- MaybeRef = clang::FileEntryRef(std::forward<Args>(args)...);
- }
+ : StorageImpl(in_place_t{}, std::forward<ArgTypes>(Args)...) {}
OptionalStorage &operator=(clang::FileEntryRef Ref) {
- MaybeRef = Ref;
+ StorageImpl::operator=(Ref);
return *this;
}
};
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 RefTy> class MapEntryOptionalStorage;
+
+} // end namespace FileMgr
/// Cached information about one directory (either on disk or in
/// the virtual file system).
@@ -37,20 +42,122 @@
/// 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<llvm::ErrorOr<DirectoryEntry &>>;
- 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<DirectoryEntryRef>;
+ struct optional_none_tag {};
- DirectoryEntryRef(
- llvm::StringMapEntry<llvm::ErrorOr<DirectoryEntry &>> *Entry)
- : Entry(Entry) {}
+ // Private constructor for use by OptionalStorage.
+ DirectoryEntryRef(optional_none_tag) : ME(nullptr) {}
+ bool hasOptionalValue() const { return ME; }
- const llvm::StringMapEntry<llvm::ErrorOr<DirectoryEntry &>> *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 RefTy> class MapEntryOptionalStorage {
+ using optional_none_tag = typename RefTy::optional_none_tag;
+ RefTy MaybeRef = optional_none_tag{};
+
+public:
+ ~MapEntryOptionalStorage() = default;
+ constexpr MapEntryOptionalStorage() noexcept = default;
+ constexpr MapEntryOptionalStorage(MapEntryOptionalStorage const &Other) =
+ default;
+ constexpr MapEntryOptionalStorage(MapEntryOptionalStorage &&Other) = default;
+ MapEntryOptionalStorage &
+ operator=(MapEntryOptionalStorage const &Other) = default;
+ MapEntryOptionalStorage &operator=(MapEntryOptionalStorage &&Other) = default;
+
+ template <class... ArgTypes>
+ constexpr explicit MapEntryOptionalStorage(llvm::optional_detail::in_place_t,
+ ArgTypes &&...Args)
+ : MaybeRef(std::forward<ArgTypes>(Args)...) {}
+
+ void reset() noexcept { MaybeRef = optional_none_tag(); }
+
+ constexpr bool hasValue() const noexcept {
+ return MaybeRef.hasOptionalValue();
+ }
+
+ RefTy &getValue() LLVM_LVALUE_FUNCTION noexcept {
+ assert(hasValue());
+ return MaybeRef;
+ }
+ constexpr RefTy const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
+ assert(hasValue());
+ return MaybeRef;
+ }
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+ RefTy &&getValue() &&noexcept {
+ assert(hasValue());
+ return std::move(MaybeRef);
+ }
+#endif
+
+ template <class... Args> void emplace(Args &&...args) {
+ MaybeRef = RefTy(std::forward<Args>(args)...);
+ }
+
+ MapEntryOptionalStorage &operator=(RefTy Ref) {
+ MaybeRef = Ref;
+ return *this;
+ }
+};
+
+} // end namespace FileMgr
} // end namespace clang
+namespace llvm {
+namespace optional_detail {
+
+/// Customize OptionalStorage<DirectoryEntryRef> to use DirectoryEntryRef and its
+/// optional_none_tag to keep it the size of a single pointer.
+template <>
+class OptionalStorage<clang::DirectoryEntryRef>
+ : public clang::FileMgr::MapEntryOptionalStorage<clang::DirectoryEntryRef> {
+ using StorageImpl =
+ clang::FileMgr::MapEntryOptionalStorage<clang::DirectoryEntryRef>;
+
+public:
+ ~OptionalStorage() = default;
+ constexpr OptionalStorage() noexcept = default;
+ constexpr OptionalStorage(OptionalStorage const &Other) = default;
+ constexpr OptionalStorage(OptionalStorage &&Other) = default;
+ OptionalStorage &operator=(OptionalStorage const &Other) = default;
+ OptionalStorage &operator=(OptionalStorage &&Other) = default;
+
+ template <class... ArgTypes>
+ constexpr explicit OptionalStorage(in_place_t, ArgTypes &&...Args)
+ : StorageImpl(in_place_t{}, std::forward<ArgTypes>(Args)...) {}
+
+ OptionalStorage &operator=(clang::DirectoryEntryRef Ref) {
+ StorageImpl::operator=(Ref);
+ return *this;
+ }
+};
+
+static_assert(sizeof(Optional<clang::DirectoryEntryRef>) ==
+ sizeof(clang::DirectoryEntryRef),
+ "Optional<DirectoryEntryRef> must avoid size overhead");
+
+static_assert(std::is_trivially_copyable<Optional<clang::DirectoryEntryRef>>::value,
+ "Optional<DirectoryEntryRef> should be trivially copyable");
+
+} // end namespace optional_detail
+} // end namespace llvm
+
#endif // LLVM_CLANG_BASIC_DIRECTORYENTRY_H
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
