dexonsmith updated this revision to Diff 299506.
dexonsmith added a comment.
Attaching the correct diff :/.
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
@@ -334,6 +334,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,131 @@
+//===- 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, MaybeFileEntryRef) {
+ MapType Refs;
+ FileEntry E1, E2;
+ MaybeFileEntryRef M0;
+ MaybeFileEntryRef M1 = addRef(Refs, "1", E1);
+ MaybeFileEntryRef M2 = addRef(Refs, "2", E2);
+ MaybeFileEntryRef M1Also = addRef(Refs, "1-also", E1);
+
+ EXPECT_EQ(None, M0.getName());
+ EXPECT_EQ(StringRef("1"), M1.getName());
+ EXPECT_EQ(StringRef("2"), M2.getName());
+ EXPECT_EQ(StringRef("1-also"), M1Also.getName());
+ EXPECT_NE(None, M1.getName());
+ EXPECT_NE(StringRef("1"), M2.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);
+
+ MaybeFileEntryRef M0;
+ MaybeFileEntryRef M1 = R1;
+ MaybeFileEntryRef M2 = R2;
+ MaybeFileEntryRef M1Also = R1Also;
+
+ EXPECT_EQ(M0, nullptr);
+ EXPECT_EQ(M0, None);
+ EXPECT_EQ(nullptr, M0);
+ EXPECT_EQ(None, M0);
+ EXPECT_EQ(M1, &E1);
+ EXPECT_EQ(&E1, M1);
+ EXPECT_EQ(R1, M1);
+ EXPECT_EQ(M1, R1);
+ EXPECT_EQ(M1, M1Also);
+ EXPECT_NE(M1, nullptr);
+ EXPECT_NE(M1, None);
+ EXPECT_NE(nullptr, M1);
+ EXPECT_NE(None, M1);
+ EXPECT_NE(M1, &E2);
+ EXPECT_NE(&E2, M1);
+ EXPECT_NE(M1, R2);
+ EXPECT_NE(R2, M1);
+ EXPECT_NE(M1, M2);
+}
+
+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_FALSE(R1.isSameRef(R2));
+ EXPECT_FALSE(R1.isSameRef(R1Also));
+
+ MaybeFileEntryRef M0;
+ MaybeFileEntryRef M1 = R1;
+ MaybeFileEntryRef M2 = R2;
+ MaybeFileEntryRef M1Also = R1Also;
+
+ EXPECT_FALSE(M0.isSameRef(M1));
+ EXPECT_FALSE(M1.isSameRef(M0));
+ EXPECT_TRUE(M1.isSameRef(MaybeFileEntryRef(M1)));
+ EXPECT_FALSE(M1.isSameRef(M1Also));
+ EXPECT_FALSE(M1.isSameRef(M2));
+ EXPECT_TRUE(M1.isSameRef(R1));
+}
+
+} // 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
SourceManagerTest.cpp
)
Index: clang/include/clang/Basic/FileEntry.h
===================================================================
--- clang/include/clang/Basic/FileEntry.h
+++ clang/include/clang/Basic/FileEntry.h
@@ -34,28 +34,8 @@
class DirectoryEntry;
class FileEntry;
-/// A reference to a \c FileEntry that includes the name of the file as it was
-/// accessed by the FileManager's client.
-class FileEntryRef {
+class FileEntryRefBase {
public:
- const StringRef getName() const { return Entry->first(); }
- const FileEntry &getFileEntry() const {
- return *Entry->second->V.get<FileEntry *>();
- }
-
- inline bool isValid() const;
- inline off_t getSize() const;
- inline unsigned getUID() const;
- inline const llvm::sys::fs::UniqueID &getUniqueID() const;
- inline time_t getModificationTime() const;
-
- friend bool operator==(const FileEntryRef &LHS, const FileEntryRef &RHS) {
- return LHS.Entry == RHS.Entry;
- }
- friend bool operator!=(const FileEntryRef &LHS, const FileEntryRef &RHS) {
- return !(LHS == RHS);
- }
-
struct MapValue;
/// Type used in the StringMap.
@@ -75,18 +55,119 @@
MapValue(MapEntry &ME) : V(&ME) {}
};
-private:
- friend class FileManager;
+ /// Check if RHS referenced the file in exactly the same way.
+ bool isSameRef(const FileEntryRefBase &RHS) const { return ME == RHS.ME; }
- 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");
+ /// 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 FileEntryRefBase &LHS,
+ const FileEntryRefBase &RHS) {
+ return LHS.getFileEntry() == RHS.getFileEntry();
+ }
+ friend bool operator==(const FileEntry *LHS, const FileEntryRefBase &RHS) {
+ return LHS == RHS.getFileEntry();
+ }
+ friend bool operator==(const FileEntryRefBase &LHS, const FileEntry *RHS) {
+ return LHS.getFileEntry() == RHS;
+ }
+ friend bool operator!=(const FileEntryRefBase &LHS,
+ const FileEntryRefBase &RHS) {
+ return !(LHS == RHS);
+ }
+ friend bool operator!=(const FileEntry *LHS, const FileEntryRefBase &RHS) {
+ return !(LHS == RHS);
+ }
+ friend bool operator!=(const FileEntryRefBase &LHS, const FileEntry *RHS) {
+ return !(LHS == RHS);
+ }
+
+protected:
+ FileEntryRefBase() = delete;
+ explicit FileEntryRefBase(const MapEntry *ME) : ME(ME) {
+ if (!ME)
+ return;
+ assert(ME->second && "Expected payload");
+ assert(ME->second->V && "Expected non-null");
+ assert(ME->second->V.is<FileEntry *>() && "Expected FileEntry");
+ }
+
+ Optional<StringRef> getName() const {
+ return ME ? ME->first() : Optional<StringRef>();
+ }
+
+ const FileEntry *getFileEntry() const {
+ return ME ? ME->second->V.get<FileEntry *>() : nullptr;
}
- const MapEntry *Entry;
+ const MapEntry *ME;
+};
+
+/// A reference to a \c FileEntry that includes the name of the file as it was
+/// accessed by the FileManager's client.
+class FileEntryRef : public FileEntryRefBase {
+ friend class MaybeFileEntryRef;
+
+public:
+ StringRef getName() const { return *FileEntryRefBase::getName(); }
+ const FileEntry &getFileEntry() const {
+ return *FileEntryRefBase::getFileEntry();
+ }
+
+ /// Allow FileEntryRef to degrade into FileEntry* to facilitate incremental
+ /// adoption.
+ operator const FileEntry *() const { return &getFileEntry(); }
+
+ inline bool isValid() const;
+ inline off_t getSize() const;
+ inline unsigned getUID() const;
+ inline const llvm::sys::fs::UniqueID &getUniqueID() const;
+ inline time_t getModificationTime() const;
+
+ explicit FileEntryRef(const MapEntry &ME) : FileEntryRefBase(&ME) {}
+};
+
+class MaybeFileEntryRef : public FileEntryRefBase {
+public:
+ using FileEntryRefBase::getFileEntry;
+ using FileEntryRefBase::getName;
+
+ /// Allow MaybeFileEntryRef to degrade into FileEntry* to facilitate
+ /// incremental adoption.
+ operator const FileEntry *() const { return getFileEntry(); }
+
+ explicit operator bool() const { return ME; }
+
+ FileEntryRef operator*() const {
+ assert(ME && "Dereferencing null?");
+ return FileEntryRef(*ME);
+ }
+
+ /// Use Optional<FileEntryRef> to provide storage for arrow.
+ Optional<FileEntryRef> operator->() const { return *this; }
+
+ friend bool operator==(llvm::NoneType, const MaybeFileEntryRef &RHS) {
+ return !RHS;
+ }
+ friend bool operator==(const MaybeFileEntryRef &LHS, llvm::NoneType) {
+ return !LHS;
+ }
+ friend bool operator!=(llvm::NoneType LHS, const MaybeFileEntryRef &RHS) {
+ return !(LHS == RHS);
+ }
+ friend bool operator!=(const MaybeFileEntryRef &LHS, llvm::NoneType RHS) {
+ return !(LHS == RHS);
+ }
+
+ /// Implicitly convert to/from Optional<FileEntryRef>.
+ operator Optional<FileEntryRef>() const {
+ return ME ? FileEntryRef(*ME) : Optional<FileEntryRef>();
+ }
+ MaybeFileEntryRef(Optional<FileEntryRef> F)
+ : FileEntryRefBase(F ? F->ME : nullptr) {}
+
+ explicit MaybeFileEntryRef(const MapEntry *ME) : FileEntryRefBase(ME) {}
+ MaybeFileEntryRef(llvm::NoneType = None) : FileEntryRefBase(nullptr) {}
+ MaybeFileEntryRef(FileEntryRef F) : FileEntryRefBase(F) {}
};
/// Cached information about one file (either on disk
@@ -115,7 +196,7 @@
// default constructor). It should always have a value in practice.
//
// TODO: remote this once everyone that needs a name uses FileEntryRef.
- Optional<FileEntryRef> LastRef;
+ MaybeFileEntryRef LastRef;
public:
FileEntry();
@@ -144,10 +225,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
