dexonsmith updated this revision to Diff 301138.
dexonsmith retitled this revision from "FileManager: Improve the FileEntryRef
API and add MaybeFileEntryRef" to "FileManager: Improve the FileEntryRef API
and customize Optional<FileEntryRef>".
dexonsmith edited the summary of this revision.
dexonsmith added a comment.
Dropped `MaybeFileEntryRef`, instead customizing `Optional<FileEntryRef>`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89834/new/
https://reviews.llvm.org/D89834
Files:
clang/include/clang/Basic/FileEntry.h
clang/unittests/Basic/CMakeLists.txt
clang/unittests/Basic/FileEntryTest.cpp
clang/unittests/Basic/FileManagerTest.cpp
Index: clang/unittests/Basic/FileManagerTest.cpp
===================================================================
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -350,6 +350,58 @@
f2 ? *f2 : nullptr);
}
+TEST_F(FileManagerTest, getFileRefEquality) {
+ auto StatCache = std::make_unique<FakeStatCache>();
+ StatCache->InjectDirectory("dir", 40);
+ StatCache->InjectFile("dir/f1.cpp", 41);
+ StatCache->InjectFile("dir/f1-also.cpp", 41);
+ StatCache->InjectFile("dir/f1-redirect.cpp", 41, "dir/f1.cpp");
+ StatCache->InjectFile("dir/f2.cpp", 42);
+ manager.setStatCache(std::move(StatCache));
+
+ auto F1 = manager.getFileRef("dir/f1.cpp");
+ auto F1Again = manager.getFileRef("dir/f1.cpp");
+ auto F1Also = manager.getFileRef("dir/f1-also.cpp");
+ auto F1Redirect = manager.getFileRef("dir/f1-redirect.cpp");
+ auto F2 = manager.getFileRef("dir/f2.cpp");
+
+ // Check Expected<FileEntryRef> for error.
+ ASSERT_FALSE(!F1);
+ ASSERT_FALSE(!F1Also);
+ ASSERT_FALSE(!F1Again);
+ ASSERT_FALSE(!F1Redirect);
+ ASSERT_FALSE(!F2);
+
+ // Check names.
+ EXPECT_EQ("dir/f1.cpp", F1->getName());
+ EXPECT_EQ("dir/f1.cpp", F1Again->getName());
+ EXPECT_EQ("dir/f1-also.cpp", F1Also->getName());
+ EXPECT_EQ("dir/f1.cpp", F1Redirect->getName());
+ EXPECT_EQ("dir/f2.cpp", F2->getName());
+
+ // Compare against FileEntry*.
+ EXPECT_EQ(&F1->getFileEntry(), *F1);
+ EXPECT_EQ(*F1, &F1->getFileEntry());
+ EXPECT_NE(&F2->getFileEntry(), *F1);
+ EXPECT_NE(*F1, &F2->getFileEntry());
+
+ // Compare using ==.
+ EXPECT_EQ(*F1, *F1Also);
+ EXPECT_EQ(*F1, *F1Again);
+ EXPECT_EQ(*F1, *F1Redirect);
+ EXPECT_EQ(*F1Also, *F1Redirect);
+ EXPECT_NE(*F2, *F1);
+ EXPECT_NE(*F2, *F1Also);
+ EXPECT_NE(*F2, *F1Again);
+ EXPECT_NE(*F2, *F1Redirect);
+
+ // Compare using isSameRef.
+ EXPECT_TRUE(F1->isSameRef(*F1Again));
+ EXPECT_TRUE(F1->isSameRef(*F1Redirect));
+ EXPECT_FALSE(F1->isSameRef(*F1Also));
+ EXPECT_FALSE(F1->isSameRef(*F2));
+}
+
// getFile() Should return the same entry as getVirtualFile if the file actually
// is a virtual file, even if the name is not exactly the same (but is after
// normalisation done by the file system, like on Windows). This can be checked
Index: clang/unittests/Basic/FileEntryTest.cpp
===================================================================
--- /dev/null
+++ clang/unittests/Basic/FileEntryTest.cpp
@@ -0,0 +1,101 @@
+//===- unittests/Basic/FileEntryTest.cpp - Test FileEntry/FileEntryRef ----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/FileEntry.h"
+#include "llvm/ADT/StringMap.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+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);
+}
+
+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);
+
+ 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());
+
+ const FileEntry *CE1 = R1;
+ EXPECT_EQ(CE1, &E1);
+}
+
+TEST(FileEntryTest, OptionalFileEntryRef) {
+ MapType Refs;
+ FileEntry E1, E2;
+ Optional<FileEntryRef> M0;
+ Optional<FileEntryRef> M1 = addRef(Refs, "1", E1);
+ Optional<FileEntryRef> M2 = addRef(Refs, "2", E2);
+ Optional<FileEntryRef> M1Also = addRef(Refs, "1-also", E1);
+
+ 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);
+}
+
+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);
+ EXPECT_EQ(R1, R1Also);
+ EXPECT_NE(R1, &E2);
+ EXPECT_NE(&E2, R1);
+ EXPECT_NE(R1, R2);
+
+ Optional<FileEntryRef> M0;
+ Optional<FileEntryRef> M1 = R1;
+
+ EXPECT_EQ(M1, &E1);
+ EXPECT_EQ(&E1, M1);
+ EXPECT_NE(M1, &E2);
+ EXPECT_NE(&E2, 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);
+
+ EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1)));
+ EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1.getMapEntry())));
+ EXPECT_FALSE(R1.isSameRef(R2));
+ EXPECT_FALSE(R1.isSameRef(R1Also));
+}
+
+} // end namespace
Index: clang/unittests/Basic/CMakeLists.txt
===================================================================
--- clang/unittests/Basic/CMakeLists.txt
+++ clang/unittests/Basic/CMakeLists.txt
@@ -5,6 +5,7 @@
add_clang_unittest(BasicTests
CharInfoTest.cpp
DiagnosticTest.cpp
+ FileEntryTest.cpp
FileManagerTest.cpp
LineOffsetMappingTest.cpp
SourceManagerTest.cpp
Index: clang/include/clang/Basic/FileEntry.h
===================================================================
--- clang/include/clang/Basic/FileEntry.h
+++ clang/include/clang/Basic/FileEntry.h
@@ -38,9 +38,9 @@
/// accessed by the FileManager's client.
class FileEntryRef {
public:
- const StringRef getName() const { return Entry->first(); }
+ StringRef getName() const { return ME->first(); }
const FileEntry &getFileEntry() const {
- return *Entry->second->V.get<FileEntry *>();
+ return *ME->second->V.get<FileEntry *>();
}
inline bool isValid() const;
@@ -49,12 +49,26 @@
inline const llvm::sys::fs::UniqueID &getUniqueID() const;
inline time_t getModificationTime() const;
+ /// Check if the underlying FileEntry is the same, intentially ignoring
+ /// whether the file was referenced with the same spelling of the filename.
friend bool operator==(const FileEntryRef &LHS, const FileEntryRef &RHS) {
- return LHS.Entry == RHS.Entry;
+ return &LHS.getFileEntry() == &RHS.getFileEntry();
+ }
+ friend bool operator==(const FileEntry *LHS, const FileEntryRef &RHS) {
+ return LHS == &RHS.getFileEntry();
+ }
+ friend bool operator==(const FileEntryRef &LHS, const FileEntry *RHS) {
+ return &LHS.getFileEntry() == RHS;
}
friend bool operator!=(const FileEntryRef &LHS, const FileEntryRef &RHS) {
return !(LHS == RHS);
}
+ friend bool operator!=(const FileEntry *LHS, const FileEntryRef &RHS) {
+ return !(LHS == RHS);
+ }
+ friend bool operator!=(const FileEntryRef &LHS, const FileEntry *RHS) {
+ return !(LHS == RHS);
+ }
struct MapValue;
@@ -75,20 +89,137 @@
MapValue(MapEntry &ME) : V(&ME) {}
};
-private:
- friend class FileManager;
+ /// Check if RHS referenced the file in exactly the same way.
+ bool isSameRef(const FileEntryRef &RHS) const { return ME == RHS.ME; }
+
+ /// Allow FileEntryRef to degrade into FileEntry* to facilitate incremental
+ /// adoption.
+ operator const FileEntry *() const { return &getFileEntry(); }
FileEntryRef() = delete;
- explicit FileEntryRef(const MapEntry &Entry)
- : Entry(&Entry) {
- assert(Entry.second && "Expected payload");
- assert(Entry.second->V && "Expected non-null");
- assert(Entry.second->V.is<FileEntry *>() && "Expected FileEntry");
+ explicit FileEntryRef(const MapEntry &ME) : ME(&ME) {
+ assert(ME.second && "Expected payload");
+ assert(ME.second->V && "Expected non-null");
+ assert(ME.second->V.is<FileEntry *>() && "Expected FileEntry");
+ }
+
+ /// Expose the underlying MapEntry to simplify packing in a PointerIntPair or
+ /// PointerUnion and allow construction in Optional.
+ const clang::FileEntryRef::MapEntry &getMapEntry() const { return *ME; }
+
+private:
+ const MapEntry *ME;
+};
+
+static_assert(sizeof(FileEntryRef) == sizeof(const FileEntry *),
+ "FileEntryRef must avoid size overhead");
+
+} // end namespace clang
+
+namespace llvm {
+
+/// Customize Optional<FileEntryRef> to use FileEntryRef::MapEntry directly
+/// in order to keep it to the size of a pointer.
+///
+/// Note that customizing OptionalStorage would not work, since the default
+/// implementation of Optional returns references and pointers to the stored
+/// value, and this customization does not store a full FileEntryRef.
+template <> class Optional<clang::FileEntryRef> {
+ const clang::FileEntryRef::MapEntry *ME = nullptr;
+
+public:
+ using value_type = clang::FileEntryRef;
+
+ Optional() {}
+ Optional(NoneType) {}
+
+ Optional(clang::FileEntryRef Ref) : ME(&Ref.getMapEntry()) {}
+ Optional(const Optional &O) = default;
+
+ /// Create a new object by constructing it in place with the given arguments.
+ template <typename... ArgTypes> void emplace(ArgTypes &&...Args) {
+ clang::FileEntryRef Ref(std::forward<ArgTypes>(Args)...);
+ *this = Ref;
}
- const MapEntry *Entry;
+ static Optional create(const clang::FileEntryRef *Ref) {
+ return Ref ? Optional(*Ref) : Optional();
+ }
+
+ Optional &operator=(clang::FileEntryRef Ref) {
+ ME = &Ref.getMapEntry();
+ return *this;
+ }
+ Optional &operator=(const Optional &O) = default;
+
+ void reset() { ME = nullptr; }
+
+ /// Type for use as a "pointer" return for the arrow operator, encapsulating
+ /// a real FileEntryRef.
+ class pointer {
+ friend class Optional;
+
+ public:
+ const clang::FileEntryRef *operator->() const { return &Ref; }
+
+ pointer() = delete;
+ pointer(const pointer &p) = default;
+
+ private:
+ explicit pointer(clang::FileEntryRef Ref) : Ref(Ref) {}
+ clang::FileEntryRef Ref;
+ };
+
+ clang::FileEntryRef getValue() const LLVM_LVALUE_FUNCTION {
+ assert(ME && "Dereferencing None?");
+ return clang::FileEntryRef(*ME);
+ }
+
+ explicit operator bool() const { return hasValue(); }
+ bool hasValue() const { return ME; }
+ pointer operator->() const { return pointer(getValue()); }
+ clang::FileEntryRef operator*() const LLVM_LVALUE_FUNCTION {
+ return getValue();
+ }
+
+ template <typename U>
+ clang::FileEntryRef getValueOr(U &&value) const LLVM_LVALUE_FUNCTION {
+ return hasValue() ? getValue() : std::forward<U>(value);
+ }
+
+ /// Apply a function to the value if present; otherwise return None.
+ template <class Function>
+ auto map(const Function &F) const LLVM_LVALUE_FUNCTION
+ -> Optional<decltype(F(getValue()))> {
+ if (*this)
+ return F(getValue());
+ return None;
+ }
+
+ /// Allow Optional<FileEntryRef> to degrade into FileEntry* to facilitate
+ /// incremental adoption of FileEntryRef. Once FileEntry::getName, remaining
+ /// uses of this should be cleaned up and this can be removed as well.
+ operator const clang::FileEntry *() const {
+ return hasValue() ? &getValue().getFileEntry() : nullptr;
+ }
+
+ /// Allow construction from the underlying pointer type to simplify
+ /// constructing from a PointerIntPair or PointerUnion.
+ explicit Optional(const clang::FileEntryRef::MapEntry *ME) : ME(ME) {}
+
+ /// Expose the underlying pointer type to simplify packing in a
+ /// PointerIntPair or PointerUnion.
+ const clang::FileEntryRef::MapEntry *getMapEntry() const { return ME; }
};
+static_assert(sizeof(Optional<clang::FileEntryRef>) ==
+ sizeof(clang::FileEntryRef),
+ "Optional<FileEntryRef> must avoid size overhead");
+
+} // end namespace llvm
+
+namespace clang {
+
/// Cached information about one file (either on disk
/// or in the virtual file system).
///
@@ -144,10 +275,6 @@
bool isNamedPipe() const { return IsNamedPipe; }
void closeFile() const;
-
- // Only for use in tests to see if deferred opens are happening, rather than
- // relying on RealPathName being empty.
- bool isOpenForTests() const { return File != nullptr; }
};
bool FileEntryRef::isValid() const { return getFileEntry().isValid(); }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
