Revert "FileManager: Improve the FileEntryRef API and customize its OptionalStorage" and follow-ups

This reverts commit 5530fb586f.
This reverts commit 010238a296.
This reverts commit 84e8257937.

Having trouble getting the bots compiling. Will try again later.
This commit is contained in:
Duncan P. N. Exon Smith 2020-10-30 14:06:15 -04:00
parent 5530fb586f
commit 940d0a310d
4 changed files with 15 additions and 369 deletions

View File

@ -31,22 +31,6 @@ class File;
namespace clang { namespace clang {
class FileEntryRef;
} // namespace clang
namespace llvm {
namespace optional_detail {
/// Forward declare a template specialization for OptionalStorage.
template <>
class OptionalStorage<clang::FileEntryRef, /*is_trivially_copyable*/ true>;
} // namespace optional_detail
} // namespace llvm
namespace clang {
class DirectoryEntry; class DirectoryEntry;
class FileEntry; class FileEntry;
@ -54,9 +38,9 @@ class FileEntry;
/// accessed by the FileManager's client. /// accessed by the FileManager's client.
class FileEntryRef { class FileEntryRef {
public: public:
StringRef getName() const { return ME->first(); } const StringRef getName() const { return Entry->first(); }
const FileEntry &getFileEntry() const { const FileEntry &getFileEntry() const {
return *ME->second->V.get<FileEntry *>(); return *Entry->second->V.get<FileEntry *>();
} }
inline bool isValid() const; inline bool isValid() const;
@ -65,26 +49,12 @@ public:
inline const llvm::sys::fs::UniqueID &getUniqueID() const; inline const llvm::sys::fs::UniqueID &getUniqueID() const;
inline time_t getModificationTime() 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) { friend bool operator==(const FileEntryRef &LHS, const FileEntryRef &RHS) {
return &LHS.getFileEntry() == &RHS.getFileEntry(); return LHS.Entry == RHS.Entry;
}
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) { friend bool operator!=(const FileEntryRef &LHS, const FileEntryRef &RHS) {
return !(LHS == 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; struct MapValue;
@ -108,191 +78,20 @@ public:
MapValue(MapEntry &ME) : V(&ME) {} MapValue(MapEntry &ME) : V(&ME) {}
}; };
/// Check if RHS referenced the file in exactly the same way. private:
bool isSameRef(const FileEntryRef &RHS) const { return ME == RHS.ME; } friend class FileManager;
/// Allow FileEntryRef to degrade into 'const FileEntry*' to facilitate
/// incremental adoption.
///
/// The goal is to avoid code churn due to dances like the following:
/// \code
/// // Old code.
/// lvalue = rvalue;
///
/// // Temporary code from an incremental patch.
/// lvalue = &rvalue.getFileEntry();
///
/// // Final code.
/// lvalue = rvalue;
/// \endcode
///
/// FIXME: Once FileEntryRef is "everywhere" and FileEntry::LastRef and
/// FileEntry::getName have been deleted, delete this implicit conversion.
operator const FileEntry *() const { return &getFileEntry(); }
FileEntryRef() = delete; FileEntryRef() = delete;
explicit FileEntryRef(const MapEntry &ME) : ME(&ME) { explicit FileEntryRef(const MapEntry &Entry)
assert(ME.second && "Expected payload"); : Entry(&Entry) {
assert(ME.second->V && "Expected non-null"); assert(Entry.second && "Expected payload");
assert(ME.second->V.is<FileEntry *>() && "Expected FileEntry"); assert(Entry.second->V && "Expected non-null");
assert(Entry.second->V.is<FileEntry *>() && "Expected FileEntry");
} }
/// Expose the underlying MapEntry to simplify packing in a PointerIntPair or const MapEntry *Entry;
/// PointerUnion and allow construction in Optional.
const clang::FileEntryRef::MapEntry &getMapEntry() const { return *ME; }
private:
friend class llvm::optional_detail::OptionalStorage<
FileEntryRef, /*is_trivially_copyable*/ true>;
struct optional_none_tag {};
// Private constructor for use by OptionalStorage.
FileEntryRef(optional_none_tag) : ME(nullptr) {}
constexpr bool hasOptionalValue() const { return ME; }
const MapEntry *ME;
}; };
static_assert(sizeof(FileEntryRef) == sizeof(const FileEntry *),
"FileEntryRef must avoid size overhead");
static_assert(std::is_trivially_copyable<FileEntryRef>::value,
"FileEntryRef must be trivially copyable");
} // end namespace clang
namespace llvm {
namespace optional_detail {
/// 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;
public:
~OptionalStorage() = default;
constexpr OptionalStorage() noexcept
: MaybeRef(clang::FileEntryRef::optional_none_tag) {}
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)
: 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)...);
}
OptionalStorage &operator=(clang::FileEntryRef Ref) {
MaybeRef = Ref;
return *this;
}
};
static_assert(sizeof(Optional<clang::FileEntryRef>) ==
sizeof(clang::FileEntryRef),
"Optional<FileEntryRef> must avoid size overhead");
static_assert(std::is_trivially_copyable<Optional<clang::FileEntryRef>>::value,
"Optional<FileEntryRef> should be trivially copyable");
} // end namespace optional_detail
} // end namespace llvm
namespace clang {
/// Wrapper around Optional<FileEntryRef> that degrades to 'const FileEntry*',
/// facilitating incremental patches to propagate FileEntryRef.
///
/// This class can be used as return value or field where it's convenient for
/// an Optional<FileEntryRef> to degrade to a 'const FileEntry*'. The purpose
/// is to avoid code churn due to dances like the following:
/// \code
/// // Old code.
/// lvalue = rvalue;
///
/// // Temporary code from an incremental patch.
/// Optional<FileEntryRef> MaybeF = rvalue;
/// lvalue = MaybeF ? &MaybeF.getFileEntry() : nullptr;
///
/// // Final code.
/// lvalue = rvalue;
/// \endcode
///
/// FIXME: Once FileEntryRef is "everywhere" and FileEntry::LastRef and
/// FileEntry::getName have been deleted, delete this class and replace
/// instances with Optional<FileEntryRef>.
class OptionalFileEntryRefDegradesToFileEntryPtr
: public Optional<FileEntryRef> {
public:
constexpr OptionalFileEntryRefDegradesToFileEntryPtr() noexcept = default;
constexpr OptionalFileEntryRefDegradesToFileEntryPtr(
OptionalFileEntryRefDegradesToFileEntryPtr &&) = default;
constexpr OptionalFileEntryRefDegradesToFileEntryPtr(
const OptionalFileEntryRefDegradesToFileEntryPtr &) = default;
OptionalFileEntryRefDegradesToFileEntryPtr &
operator=(OptionalFileEntryRefDegradesToFileEntryPtr &&) = default;
OptionalFileEntryRefDegradesToFileEntryPtr &
operator=(const OptionalFileEntryRefDegradesToFileEntryPtr &) = default;
OptionalFileEntryRefDegradesToFileEntryPtr(llvm::NoneType) {}
OptionalFileEntryRefDegradesToFileEntryPtr(FileEntryRef Ref)
: Optional<FileEntryRef>(Ref) {}
OptionalFileEntryRefDegradesToFileEntryPtr(Optional<FileEntryRef> MaybeRef)
: Optional<FileEntryRef>(MaybeRef) {}
OptionalFileEntryRefDegradesToFileEntryPtr &operator=(llvm::NoneType) {
Optional<FileEntryRef>::operator=(None);
return *this;
}
OptionalFileEntryRefDegradesToFileEntryPtr &operator=(FileEntryRef Ref) {
Optional<FileEntryRef>::operator=(Ref);
return *this;
}
OptionalFileEntryRefDegradesToFileEntryPtr &
operator=(Optional<FileEntryRef> MaybeRef) {
Optional<FileEntryRef>::operator=(MaybeRef);
return *this;
}
/// Degrade to 'const FileEntry *' to allow FileEntry::LastRef and
/// FileEntry::getName have been deleted, delete this class and replace
/// instances with Optional<FileEntryRef>
operator const FileEntry *() const {
return hasValue() ? &getValue().getFileEntry() : nullptr;
}
};
static_assert(
std::is_trivially_copyable<
OptionalFileEntryRefDegradesToFileEntryPtr>::value,
"OptionalFileEntryRefDegradesToFileEntryPtr should be trivially copyable");
/// Cached information about one file (either on disk /// Cached information about one file (either on disk
/// or in the virtual file system). /// or in the virtual file system).
/// ///
@ -348,6 +147,10 @@ public:
bool isNamedPipe() const { return IsNamedPipe; } bool isNamedPipe() const { return IsNamedPipe; }
void closeFile() const; 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(); } bool FileEntryRef::isValid() const { return getFileEntry().isValid(); }

View File

@ -5,7 +5,6 @@ set(LLVM_LINK_COMPONENTS
add_clang_unittest(BasicTests add_clang_unittest(BasicTests
CharInfoTest.cpp CharInfoTest.cpp
DiagnosticTest.cpp DiagnosticTest.cpp
FileEntryTest.cpp
FileManagerTest.cpp FileManagerTest.cpp
LineOffsetMappingTest.cpp LineOffsetMappingTest.cpp
SourceManagerTest.cpp SourceManagerTest.cpp

View File

@ -1,104 +0,0 @@
//===- 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, OptionalFileEntryRefDegradesToFileEntryPtr) {
MapType Refs;
FileEntry E1, E2;
OptionalFileEntryRefDegradesToFileEntryPtr M0;
OptionalFileEntryRefDegradesToFileEntryPtr M1 = addRef(Refs, "1", E1);
OptionalFileEntryRefDegradesToFileEntryPtr M2 = addRef(Refs, "2", E2);
OptionalFileEntryRefDegradesToFileEntryPtr M0Also = None;
OptionalFileEntryRefDegradesToFileEntryPtr M1Also =
addRef(Refs, "1-also", E1);
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);
}
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);
OptionalFileEntryRefDegradesToFileEntryPtr M0;
OptionalFileEntryRefDegradesToFileEntryPtr 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

View File

@ -350,58 +350,6 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedVirtualFiles) {
f2 ? *f2 : nullptr); 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 // 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 // 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 // normalisation done by the file system, like on Windows). This can be checked