[clang] 940d0a3 - Revert "FileManager: Improve the FileEntryRef API and customize its OptionalStorage" and follow-ups
Duncan P. N. Exon Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 30 11:07:37 PDT 2020
Author: Duncan P. N. Exon Smith
Date: 2020-10-30T14:06:55-04:00
New Revision: 940d0a310dca31ae97080b068cef92eadfee6367
URL: https://github.com/llvm/llvm-project/commit/940d0a310dca31ae97080b068cef92eadfee6367
DIFF: https://github.com/llvm/llvm-project/commit/940d0a310dca31ae97080b068cef92eadfee6367.diff
LOG: Revert "FileManager: Improve the FileEntryRef API and customize its OptionalStorage" and follow-ups
This reverts commit 5530fb586f30da9dcb434f6be39198dbf016b866.
This reverts commit 010238a296e61cbf6f4d7f4383e26cf00c4e4992.
This reverts commit 84e8257937ec6a332aa0b688f4dce57016516ffd.
Having trouble getting the bots compiling. Will try again later.
Added:
Modified:
clang/include/clang/Basic/FileEntry.h
clang/unittests/Basic/CMakeLists.txt
clang/unittests/Basic/FileManagerTest.cpp
Removed:
clang/unittests/Basic/FileEntryTest.cpp
################################################################################
diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h
index 89788b1e68f9..65149569bb59 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -31,22 +31,6 @@ 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;
@@ -54,9 +38,9 @@ class FileEntry;
/// accessed by the FileManager's client.
class FileEntryRef {
public:
- StringRef getName() const { return ME->first(); }
+ const StringRef getName() const { return Entry->first(); }
const FileEntry &getFileEntry() const {
- return *ME->second->V.get<FileEntry *>();
+ return *Entry->second->V.get<FileEntry *>();
}
inline bool isValid() const;
@@ -65,26 +49,12 @@ class FileEntryRef {
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.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;
+ return LHS.Entry == RHS.Entry;
}
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;
@@ -108,191 +78,20 @@ class FileEntryRef {
MapValue(MapEntry &ME) : V(&ME) {}
};
- /// 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 &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:
- 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 {
+ friend class FileManager;
-/// 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;
+ 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");
}
- /// 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;
- }
+ const MapEntry *Entry;
};
-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).
///
@@ -348,6 +147,10 @@ class FileEntry {
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(); }
diff --git a/clang/unittests/Basic/CMakeLists.txt b/clang/unittests/Basic/CMakeLists.txt
index add190b4ab0f..b7fa259fd1b5 100644
--- a/clang/unittests/Basic/CMakeLists.txt
+++ b/clang/unittests/Basic/CMakeLists.txt
@@ -5,7 +5,6 @@ set(LLVM_LINK_COMPONENTS
add_clang_unittest(BasicTests
CharInfoTest.cpp
DiagnosticTest.cpp
- FileEntryTest.cpp
FileManagerTest.cpp
LineOffsetMappingTest.cpp
SourceManagerTest.cpp
diff --git a/clang/unittests/Basic/FileEntryTest.cpp b/clang/unittests/Basic/FileEntryTest.cpp
deleted file mode 100644
index ce57d16d9eae..000000000000
--- a/clang/unittests/Basic/FileEntryTest.cpp
+++ /dev/null
@@ -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
diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp
index 0a1f58f3bb90..43680d5cfbed 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -350,58 +350,6 @@ 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
More information about the cfe-commits
mailing list