From e40e43d033f12cba6b6bbd88a978d10f1e79fb86 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Thu, 27 Feb 2014 00:25:12 +0000 Subject: [PATCH] Add a 'use-external-names' option to VFS overlay files When true, sets the name of the file to be the name from 'external-contents'. Otherwise, you get the virtual path that the file was looked up by. This will not affect any non-virtual paths, or fully virtual paths (for which there is no reasonable 'external' name anyway). The setting is available globally, but can be overriden on a per-file basis. The goal is that this setting will control which path you see in debug info, diagnostics, etc. which are sensitive to which path is used. That will come in future patches that pass the name through to FileManager. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@202329 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/VirtualFileSystem.h | 10 +--- lib/Basic/VirtualFileSystem.cpp | 61 +++++++++++++++++------ unittests/Basic/VirtualFileSystemTest.cpp | 54 +++++++++++++++++++- 3 files changed, 99 insertions(+), 26 deletions(-) diff --git a/include/clang/Basic/VirtualFileSystem.h b/include/clang/Basic/VirtualFileSystem.h index 1c98f677c0..bd568120b2 100644 --- a/include/clang/Basic/VirtualFileSystem.h +++ b/include/clang/Basic/VirtualFileSystem.h @@ -29,7 +29,6 @@ namespace vfs { /// \brief The result of a \p status operation. class Status { std::string Name; - std::string ExternalName; llvm::sys::fs::UniqueID UID; llvm::sys::TimeValue MTime; uint32_t User; @@ -46,16 +45,9 @@ public: uint64_t Size, llvm::sys::fs::file_type Type, llvm::sys::fs::perms Perms); - /// \brief Returns the name this status was looked up by. + /// \brief Returns the name that should be used for this file or directory. StringRef getName() const { return Name; } - - /// \brief Returns the name to use outside the compiler. - /// - /// For example, in diagnostics or debug info we should use this name. - StringRef getExternalName() const { return ExternalName; } - void setName(StringRef N) { Name = N; } - void setExternalName(StringRef N) { ExternalName = N; } /// @name Status interface from llvm::sys::fs /// @{ diff --git a/lib/Basic/VirtualFileSystem.cpp b/lib/Basic/VirtualFileSystem.cpp index 539cbb7409..f6d88c1860 100644 --- a/lib/Basic/VirtualFileSystem.cpp +++ b/lib/Basic/VirtualFileSystem.cpp @@ -35,8 +35,8 @@ Status::Status(const file_status &Status) Status::Status(StringRef Name, StringRef ExternalName, UniqueID UID, sys::TimeValue MTime, uint32_t User, uint32_t Group, uint64_t Size, file_type Type, perms Perms) - : Name(Name), ExternalName(ExternalName), UID(UID), MTime(MTime), - User(User), Group(Group), Size(Size), Type(Type), Perms(Perms) {} + : Name(Name), UID(UID), MTime(MTime), User(User), Group(Group), Size(Size), + Type(Type), Perms(Perms) {} bool Status::equivalent(const Status &Other) const { return getUniqueID() == Other.getUniqueID(); @@ -145,7 +145,6 @@ ErrorOr RealFileSystem::status(const Twine &Path) { return EC; Status Result(RealStatus); Result.setName(Path.str()); - Result.setExternalName(Path.str()); return Result; } @@ -253,12 +252,22 @@ public: }; class FileEntry : public Entry { +public: + enum NameKind { + NK_NotSet, + NK_External, + NK_Virtual + }; +private: std::string ExternalContentsPath; - + NameKind UseName; public: - FileEntry(StringRef Name, StringRef ExternalContentsPath) - : Entry(EK_File, Name), ExternalContentsPath(ExternalContentsPath) {} + FileEntry(StringRef Name, StringRef ExternalContentsPath, NameKind UseName) + : Entry(EK_File, Name), ExternalContentsPath(ExternalContentsPath), + UseName(UseName) {} StringRef getExternalContentsPath() const { return ExternalContentsPath; } + /// \brief whether to use the external path as the name for this file. + NameKind useName() const { return UseName; } static bool classof(const Entry *E) { return E->getKind() == EK_File; } }; @@ -280,6 +289,7 @@ public: /// /// All configuration options are optional. /// 'case-sensitive': +/// 'use-external-names': /// /// Virtual directories are represented as /// \verbatim @@ -304,6 +314,7 @@ public: /// { /// 'type': 'file', /// 'name': , +/// 'use-external-name': # Optional /// 'external-contents': ) /// } /// \endverbatim @@ -324,14 +335,18 @@ class VFSFromYAML : public vfs::FileSystem { /// \brief Whether to perform case-sensitive comparisons. /// /// Currently, case-insensitive matching only works correctly with ASCII. - bool CaseSensitive; ///< Whether to perform case-sensitive comparisons. + bool CaseSensitive; + + /// \brief Whether to use to use the value of 'external-contents' for the + /// names of files. This global value is overridable on a per-file basis. + bool UseExternalNames; /// @} friend class VFSFromYAMLParser; private: VFSFromYAML(IntrusiveRefCntPtr ExternalFS) - : ExternalFS(ExternalFS), CaseSensitive(true) {} + : ExternalFS(ExternalFS), CaseSensitive(true), UseExternalNames(true) {} /// \brief Looks up \p Path in \c Roots. ErrorOr lookupPath(const Twine &Path); @@ -446,7 +461,8 @@ class VFSFromYAMLParser { KeyStatusPair("name", true), KeyStatusPair("type", true), KeyStatusPair("contents", false), - KeyStatusPair("external-contents", false) + KeyStatusPair("external-contents", false), + KeyStatusPair("use-external-name", false), }; DenseMap Keys( @@ -456,6 +472,7 @@ class VFSFromYAMLParser { std::vector EntryArrayContents; std::string ExternalContentsPath; std::string Name; + FileEntry::NameKind UseExternalName = FileEntry::NK_NotSet; EntryKind Kind; for (yaml::MappingNode::iterator I = M->begin(), E = M->end(); I != E; @@ -519,6 +536,11 @@ class VFSFromYAMLParser { if (!parseScalarString(I->getValue(), Value, Buffer)) return NULL; ExternalContentsPath = Value; + } else if (Key == "use-external-name") { + bool Val; + if (!parseScalarBool(I->getValue(), Val)) + return NULL; + UseExternalName = Val ? FileEntry::NK_External : FileEntry::NK_Virtual; } else { llvm_unreachable("key missing from Keys"); } @@ -535,6 +557,12 @@ class VFSFromYAMLParser { if (!checkMissingKeys(N, Keys)) return NULL; + // check invalid configuration + if (Kind == EK_Directory && UseExternalName != FileEntry::NK_NotSet) { + error(N, "'use-external-name' is not supported for directories"); + return NULL; + } + // Remove trailing slash(es) StringRef Trimmed(Name); while (Trimmed.size() > 1 && sys::path::is_separator(Trimmed.back())) @@ -545,7 +573,8 @@ class VFSFromYAMLParser { Entry *Result = 0; switch (Kind) { case EK_File: - Result = new FileEntry(LastComponent, llvm_move(ExternalContentsPath)); + Result = new FileEntry(LastComponent, llvm_move(ExternalContentsPath), + UseExternalName); break; case EK_Directory: Result = new DirectoryEntry(LastComponent, llvm_move(EntryArrayContents), @@ -583,6 +612,7 @@ public: KeyStatusPair Fields[] = { KeyStatusPair("version", true), KeyStatusPair("case-sensitive", false), + KeyStatusPair("use-external-names", false), KeyStatusPair("roots", true), }; @@ -635,6 +665,9 @@ public: } else if (Key == "case-sensitive") { if (!parseScalarBool(I->getValue(), FS->CaseSensitive)) return false; + } else if (Key == "use-external-names") { + if (!parseScalarBool(I->getValue(), FS->UseExternalNames)) + return false; } else { llvm_unreachable("key missing from Keys"); } @@ -736,17 +769,15 @@ ErrorOr VFSFromYAML::status(const Twine &Path) { std::string PathStr(Path.str()); if (FileEntry *F = dyn_cast(*Result)) { ErrorOr S = ExternalFS->status(F->getExternalContentsPath()); - if (S) { - assert(S->getName() == S->getExternalName() && - S->getName() == F->getExternalContentsPath()); + assert(!S || S->getName() == F->getExternalContentsPath()); + if (S && (F->useName() == FileEntry::NK_Virtual || + (F->useName() == FileEntry::NK_NotSet && !UseExternalNames))) S->setName(PathStr); - } return S; } else { // directory DirectoryEntry *DE = cast(*Result); Status S = DE->getStatus(); S.setName(PathStr); - S.setExternalName(PathStr); return S; } } diff --git a/unittests/Basic/VirtualFileSystemTest.cpp b/unittests/Basic/VirtualFileSystemTest.cpp index 334b1a45dd..367e79492a 100644 --- a/unittests/Basic/VirtualFileSystemTest.cpp +++ b/unittests/Basic/VirtualFileSystemTest.cpp @@ -290,8 +290,7 @@ TEST_F(VFSFromYAMLTest, MappedFiles) { // file ErrorOr S = O->status("/file1"); ASSERT_EQ(errc::success, S.getError()); - EXPECT_EQ("/file1", S->getName()); - EXPECT_EQ("/foo/bar/a", S->getExternalName()); + EXPECT_EQ("/foo/bar/a", S->getName()); ErrorOr SLower = O->status("/foo/bar/a"); EXPECT_EQ("/foo/bar/a", SLower->getName()); @@ -467,6 +466,57 @@ TEST_F(VFSFromYAMLTest, IllegalVFSFile) { EXPECT_EQ(24, NumDiagnostics); } +TEST_F(VFSFromYAMLTest, UseExternalName) { + IntrusiveRefCntPtr Lower(new DummyFileSystem()); + Lower->addRegularFile("/external/file"); + + IntrusiveRefCntPtr FS = getFromYAMLString( + "{ 'roots': [\n" + " { 'type': 'file', 'name': '/A',\n" + " 'external-contents': '/external/file'\n" + " },\n" + " { 'type': 'file', 'name': '/B',\n" + " 'use-external-name': true,\n" + " 'external-contents': '/external/file'\n" + " },\n" + " { 'type': 'file', 'name': '/C',\n" + " 'use-external-name': false,\n" + " 'external-contents': '/external/file'\n" + " }\n" + "] }", Lower); + ASSERT_TRUE(NULL != FS.getPtr()); + + // default true + EXPECT_EQ("/external/file", FS->status("/A")->getName()); + // explicit + EXPECT_EQ("/external/file", FS->status("/B")->getName()); + EXPECT_EQ("/C", FS->status("/C")->getName()); + + // global configuration + FS = getFromYAMLString( + "{ 'use-external-names': false,\n" + " 'roots': [\n" + " { 'type': 'file', 'name': '/A',\n" + " 'external-contents': '/external/file'\n" + " },\n" + " { 'type': 'file', 'name': '/B',\n" + " 'use-external-name': true,\n" + " 'external-contents': '/external/file'\n" + " },\n" + " { 'type': 'file', 'name': '/C',\n" + " 'use-external-name': false,\n" + " 'external-contents': '/external/file'\n" + " }\n" + "] }", Lower); + ASSERT_TRUE(NULL != FS.getPtr()); + + // default + EXPECT_EQ("/A", FS->status("/A")->getName()); + // explicit + EXPECT_EQ("/external/file", FS->status("/B")->getName()); + EXPECT_EQ("/C", FS->status("/C")->getName()); +} + TEST_F(VFSFromYAMLTest, MultiComponentPath) { IntrusiveRefCntPtr Lower(new DummyFileSystem()); Lower->addRegularFile("/other"); -- 2.40.0