forked from OSchip/llvm-project
FileManager: Improve the FileEntryRef API and customize its OptionalStorage
Make a few changes to the `FileEntryRef` API in preparation for propagating it enough to remove `FileEntry::getName()`. - Allow `FileEntryRef` to degrade implicitly to `const FileEntry*`. This allows functions currently returning `const FileEntry *` to be updated to return `FileEntryRef` without requiring all callers to be updated in the same patch. This helps avoid both (a) massive patches where many fields and locals are updated simultaneously and (b) noisy incremental patches where the first patch adds `getFileEntry()` at call sites and the second patch removes it. (Once `FileEntryRef` is everywhere, we should remove this API.) - Change `operator==` to compare the underlying `FileEntry*`, ignoring any difference in the spelling of the filename. There were 0 users of the existing function because it's not useful. In case comparing the exact named reference becomes important, add/test `isSameRef`. - Add `==` comparisons between `FileEntryRef` and `const FileEntry *` (compares the `FileEntry*`). - Customize `OptionalStorage<FileEntryRef>` to be pointer-sized. Add a private constructor that initializes with `nullptr` and specialize `OptionalStorage` to use it. This unblocks updating fields in size-sensitive data structures that currently use `const FileEntry *`. - Add `OptionalFileEntryRefDegradesToFileEntryPtr`, a wrapper around `Optional<FileEntryRef>` that degrades to `const FileEntry*`. This facilitates future incremental patches, like the same operator on `FileEntryRef`. (Once `FileEntryRef` is everywhere, we should remove this class.) - Remove the unncessary `const` from the by-value return of `FileEntryRef::getName`. - Delete the unused function `FileEntry::isOpenForTests`. Note that there are still `FileEntry` APIs that aren't wrapped and I plan to deal with these separately / incrementally, as they are needed. Differential Revision: https://reviews.llvm.org/D89834
This commit is contained in:
Duncan P. N. Exon Smith
parent
2177e4555a
commit
84e8257937
|
|
@ -31,6 +31,22 @@ class File;
|
|||
|
||||
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 FileEntry;
|
||||
|
||||
|
|
@ -38,9 +54,9 @@ class FileEntry;
|
|||
/// 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 +65,26 @@ public:
|
|||
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;
|
||||
|
||||
|
|
@ -78,20 +108,190 @@ public:
|
|||
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 '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;
|
||||
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");
|
||||
}
|
||||
|
||||
const MapEntry *Entry;
|
||||
/// 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:
|
||||
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) {}
|
||||
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 = clang::FileEntryRef::optional_none_tag{};
|
||||
|
||||
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)
|
||||
: 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
|
||||
/// or in the virtual file system).
|
||||
///
|
||||
|
|
@ -147,10 +347,6 @@ public:
|
|||
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(); }
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
|
|||
add_clang_unittest(BasicTests
|
||||
CharInfoTest.cpp
|
||||
DiagnosticTest.cpp
|
||||
FileEntryTest.cpp
|
||||
FileManagerTest.cpp
|
||||
LineOffsetMappingTest.cpp
|
||||
SourceManagerTest.cpp
|
||||
|
|
|
|||
|
|
@ -0,0 +1,104 @@
|
|||
//===- 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
|
||||
|
|
@ -350,6 +350,58 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedVirtualFiles) {
|
|||
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
|
||||
|
|
|
|||
Loading…
Reference in New Issue