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.

llvm-svn: 202329

3 files changed, 99 insertions, 26 deletions

diff --git a/clang/include/clang/Basic/VirtualFileSystem.h b/clang/include/clang/Basic/VirtualFileSystem.h
index 1c98f677c01..bd568120b25 100644
--- a/clang/include/clang/Basic/VirtualFileSystem.h
+++ b/clang/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/clang/lib/Basic/VirtualFileSystem.cpp b/clang/lib/Basic/VirtualFileSystem.cpp
index 539cbb74097..f6d88c1860d 100644
--- a/clang/lib/Basic/VirtualFileSystem.cpp
+++ b/clang/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<Status> 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': <boolean, default=true>
+///   'use-external-names': <boolean, default=true>
 ///
 /// Virtual directories are represented as
 /// \verbatim
@@ -304,6 +314,7 @@ public:
 /// {
 ///   'type': 'file',
 ///   'name': <string>,
+///   'use-external-name': <boolean> # Optional
 ///   'external-contents': <path to external file>)
 /// }
 /// \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<FileSystem> ExternalFS)
-      : ExternalFS(ExternalFS), CaseSensitive(true) {}
+      : ExternalFS(ExternalFS), CaseSensitive(true), UseExternalNames(true) {}
 
   /// \brief Looks up \p Path in \c Roots.
   ErrorOr<Entry *> 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<StringRef, KeyStatus> Keys(
@@ -456,6 +472,7 @@ class VFSFromYAMLParser {
     std::vector<Entry *> 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<Status> VFSFromYAML::status(const Twine &Path) {
   std::string PathStr(Path.str());
   if (FileEntry *F = dyn_cast<FileEntry>(*Result)) {
     ErrorOr<Status> 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<DirectoryEntry>(*Result);
     Status S = DE->getStatus();
     S.setName(PathStr);
-    S.setExternalName(PathStr);
     return S;
   }
 }
diff --git a/clang/unittests/Basic/VirtualFileSystemTest.cpp b/clang/unittests/Basic/VirtualFileSystemTest.cpp
index 334b1a45ddf..367e79492a8 100644
--- a/clang/unittests/Basic/VirtualFileSystemTest.cpp
+++ b/clang/unittests/Basic/VirtualFileSystemTest.cpp
@@ -290,8 +290,7 @@ TEST_F(VFSFromYAMLTest, MappedFiles) {
   // file
   ErrorOr<vfs::Status> 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<vfs::Status> 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<DummyFileSystem> Lower(new DummyFileSystem());
+  Lower->addRegularFile("/external/file");
+
+  IntrusiveRefCntPtr<vfs::FileSystem> 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<DummyFileSystem> Lower(new DummyFileSystem());
   Lower->addRegularFile("/other");