Reapply fixed "Honour 'use-external-names' in FileManager"

Was r202442

There were two issues with the original patch that have now been fixed.
1. We were memset'ing over a FileEntry in a test case. After adding a
   std::string to FileEntry, this still happened to not break for me.
2. I didn't pass the FileManager into the new compiler instance in
   compileModule. This was hidden in some cases by the fact I didn't
   clear the module cache in the test.

Also, I changed the copy constructor for FileEntry, which was memcpy'ing
in a (now) unsafe way.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@202539 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/FileManager.h b/include/clang/Basic/FileManager.h
index b5b7231e13c36cddc2621914cf82928d266b8300..1e13442770ee9db4fcc2376db9d851a8efae30e4 100644 (file)
--- a/include/clang/Basic/FileManager.h
+++ b/include/clang/Basic/FileManager.h
@@ -59,7 +59,7 @@ public:
 /// If the 'File' member is valid, then this FileEntry has an open file
 /// descriptor for the file.
 class FileEntry {
-  const char *Name;           // Name of the file.
+  std::string Name;           // Name of the file.
   off_t Size;                 // File size in bytes.
   time_t ModTime;             // Modification time of file.
   const DirectoryEntry *Dir;  // Directory file lives in.
@@ -81,18 +81,19 @@ class FileEntry {
 
 public:
   FileEntry()
-      : Name(0), UniqueID(0, 0), IsNamedPipe(false), InPCH(false),
-        IsValid(false)
+      : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false)
   {}
 
   // FIXME: this is here to allow putting FileEntry in std::map.  Once we have
   // emplace, we shouldn't need a copy constructor anymore.
-  FileEntry(const FileEntry &FE) {
-    memcpy(this, &FE, sizeof(FE));
+  /// Intentionally does not copy fields that are not set in an uninitialized
+  /// \c FileEntry.
+  FileEntry(const FileEntry &FE) : UniqueID(FE.UniqueID),
+      IsNamedPipe(FE.IsNamedPipe), InPCH(FE.InPCH), IsValid(FE.IsValid) {
     assert(!isValid() && "Cannot copy an initialized FileEntry");
   }
 
-  const char *getName() const { return Name; }
+  const char *getName() const { return Name.c_str(); }
   bool isValid() const { return IsValid; }
   off_t getSize() const { return Size; }
   unsigned getUID() const { return UID; }

diff --git a/include/clang/Basic/FileSystemStatCache.h b/include/clang/Basic/FileSystemStatCache.h
index 913e41e4ca8be33941eefb920d9e7ded916c4e72..7ed0acc7a8c20abe7f8cb5a1ae37484c3581c92a 100644 (file)
--- a/include/clang/Basic/FileSystemStatCache.h
+++ b/include/clang/Basic/FileSystemStatCache.h
@@ -29,7 +29,9 @@ class File;
 class FileSystem;
 }
 
+// FIXME: should probably replace this with vfs::Status
 struct FileData {
+  std::string Name;
   uint64_t Size;
   time_t ModTime;
   llvm::sys::fs::UniqueID UniqueID;

diff --git a/include/clang/Basic/VirtualFileSystem.h b/include/clang/Basic/VirtualFileSystem.h
index bd568120b25bce64f128bede8fcaa7fc2687ce8b..8f144da917f0ed8de61104685637da24e27f6c28 100644 (file)
--- a/include/clang/Basic/VirtualFileSystem.h
+++ b/include/clang/Basic/VirtualFileSystem.h
@@ -90,6 +90,8 @@ public:
                                      bool RequiresNullTerminator = true) = 0;
   /// \brief Closes the file.
   virtual llvm::error_code close() = 0;
+  /// \brief Sets the name to use for this file.
+  virtual void setName(StringRef Name) = 0;
 };
 
 /// \brief The virtual file system interface.

diff --git a/lib/Basic/FileManager.cpp b/lib/Basic/FileManager.cpp
index f7e566b977981df1b5c6ebba067451145d60a4f6..5784f31843f30ae703e3ebe8f367bcb383acacf5 100644 (file)
--- a/lib/Basic/FileManager.cpp
+++ b/lib/Basic/FileManager.cpp
@@ -282,7 +282,7 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
   }
 
   // Otherwise, we don't have this file yet, add it.
-  UFE.Name    = InterndFileName;
+  UFE.Name    = Data.Name;
   UFE.Size = Data.Size;
   UFE.ModTime = Data.ModTime;
   UFE.Dir     = DirInfo;

diff --git a/lib/Basic/FileSystemStatCache.cpp b/lib/Basic/FileSystemStatCache.cpp
index b225facbad9592269a7e983a1ae48bb4983b9985..e50dc1b5ed0300ec49538b72077f27f3ce21a18a 100644 (file)
--- a/lib/Basic/FileSystemStatCache.cpp
+++ b/lib/Basic/FileSystemStatCache.cpp
@@ -32,6 +32,7 @@ void FileSystemStatCache::anchor() { }
 
 static void copyStatusToFileData(const vfs::Status &Status,
                                  FileData &Data) {
+  Data.Name = Status.getName();
   Data.Size = Status.getSize();
   Data.ModTime = Status.getLastModificationTime().toEpochTime();
   Data.UniqueID = Status.getUniqueID();

diff --git a/lib/Basic/VirtualFileSystem.cpp b/lib/Basic/VirtualFileSystem.cpp
index f6d88c1860d10263f39a3845a24ce1caecf32073..d4845e6f363b746b80a4d70c341400a69c964f4d 100644 (file)
--- a/lib/Basic/VirtualFileSystem.cpp
+++ b/lib/Basic/VirtualFileSystem.cpp
@@ -83,6 +83,7 @@ error_code FileSystem::getBufferForFile(const llvm::Twine &Name,
 /// \brief Wrapper around a raw file descriptor.
 class RealFile : public File {
   int FD;
+  Status S;
   friend class RealFileSystem;
   RealFile(int FD) : FD(FD) {
     assert(FD >= 0 && "Invalid or inactive file descriptor");
@@ -95,15 +96,21 @@ public:
                        int64_t FileSize = -1,
                        bool RequiresNullTerminator = true) LLVM_OVERRIDE;
   error_code close() LLVM_OVERRIDE;
+  void setName(StringRef Name) LLVM_OVERRIDE;
 };
 RealFile::~RealFile() { close(); }
 
 ErrorOr<Status> RealFile::status() {
   assert(FD != -1 && "cannot stat closed file");
-  file_status RealStatus;
-  if (error_code EC = sys::fs::status(FD, RealStatus))
-    return EC;
-  return Status(RealStatus);
+  if (!S.isStatusKnown()) {
+    file_status RealStatus;
+    if (error_code EC = sys::fs::status(FD, RealStatus))
+      return EC;
+    Status NewS(RealStatus);
+    NewS.setName(S.getName());
+    S = llvm_move(NewS);
+  }
+  return S;
 }
 
 error_code RealFile::getBuffer(const Twine &Name,
@@ -131,6 +138,10 @@ error_code RealFile::close() {
   return error_code::success();
 }
 
+void RealFile::setName(StringRef Name) {
+  S.setName(Name);
+}
+
 /// \brief The file system according to your operating system.
 class RealFileSystem : public FileSystem {
 public:
@@ -154,6 +165,7 @@ error_code RealFileSystem::openFileForRead(const Twine &Name,
   if (error_code EC = sys::fs::openFileForRead(Name, FD))
     return EC;
   Result.reset(new RealFile(FD));
+  Result->setName(Name.str());
   return error_code::success();
 }
 
@@ -267,7 +279,10 @@ public:
         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; }
+  bool useExternalName(bool GlobalUseExternalName) const {
+    return UseName == NK_NotSet ? GlobalUseExternalName
+                                : (UseName == NK_External);
+  }
   static bool classof(const Entry *E) { return E->getKind() == EK_File; }
 };
 
@@ -770,8 +785,7 @@ ErrorOr<Status> VFSFromYAML::status(const Twine &Path) {
   if (FileEntry *F = dyn_cast<FileEntry>(*Result)) {
     ErrorOr<Status> S = ExternalFS->status(F->getExternalContentsPath());
     assert(!S || S->getName() == F->getExternalContentsPath());
-    if (S && (F->useName() == FileEntry::NK_Virtual ||
-              (F->useName() == FileEntry::NK_NotSet && !UseExternalNames)))
+    if (S && !F->useExternalName(UseExternalNames))
       S->setName(PathStr);
     return S;
   } else { // directory
@@ -792,7 +806,14 @@ error_code VFSFromYAML::openFileForRead(const Twine &Path,
   if (!F) // FIXME: errc::not_a_file?
     return error_code(errc::invalid_argument, system_category());
 
-  return ExternalFS->openFileForRead(F->getExternalContentsPath(), Result);
+  if (error_code EC = ExternalFS->openFileForRead(F->getExternalContentsPath(),
+                                                  Result))
+    return EC;
+
+  if (!F->useExternalName(UseExternalNames))
+    Result->setName(Path.str());
+
+  return error_code::success();
 }
 
 IntrusiveRefCntPtr<FileSystem>

diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp
index cdc4b64af77376bac69d2bb72ad177e3969a77b3..a2b321ff6c9c65b57c7627dc051b62ca6044ea68 100644 (file)
--- a/lib/Frontend/CompilerInstance.cpp
+++ b/lib/Frontend/CompilerInstance.cpp
@@ -886,7 +886,7 @@ static void compileModule(CompilerInstance &ImportingInstance,
 
   // Note that this module is part of the module build stack, so that we
   // can detect cycles in the module graph.
-  Instance.createFileManager(); // FIXME: Adopt file manager from importer?
+  Instance.setFileManager(&ImportingInstance.getFileManager());
   Instance.createSourceManager(Instance.getFileManager());
   SourceManager &SourceMgr = Instance.getSourceManager();
   SourceMgr.setModuleBuildStack(

diff --git a/lib/Lex/PTHLexer.cpp b/lib/Lex/PTHLexer.cpp
index cdc5d7e338165bffcb3ad10b189ca784f8a9e682..dd8363df9cafe03359ef21516a3a7096dd4d7a69 100644 (file)
--- a/lib/Lex/PTHLexer.cpp
+++ b/lib/Lex/PTHLexer.cpp
@@ -688,6 +688,7 @@ public:
     if (!D.HasData)
       return CacheMissing;
 
+    Data.Name = Path;
     Data.Size = D.Size;
     Data.ModTime = D.ModTime;
     Data.UniqueID = D.UniqueID;

diff --git a/test/VFS/Inputs/external-names.h b/test/VFS/Inputs/external-names.h
new file mode 100644 (file)
index 0000000..8b0baa3
--- /dev/null
+++ b/test/VFS/Inputs/external-names.h
@@ -0,0 +1,4 @@
+void foo(char **c) {
+  *c = __FILE__;
+  int x = c; // produce a diagnostic
+}

diff --git a/test/VFS/Inputs/use-external-names.yaml b/test/VFS/Inputs/use-external-names.yaml
new file mode 100644 (file)
index 0000000..b9ea634
--- /dev/null
+++ b/test/VFS/Inputs/use-external-names.yaml
@@ -0,0 +1,7 @@
+{
+  'version': 0,
+  'use-external-names': EXTERNAL_NAMES,
+  'roots': [{ 'type': 'file', 'name': 'OUT_DIR/external-names.h',
+             'external-contents': 'INPUT_DIR/external-names.h'
+           }]
+}

diff --git a/test/VFS/external-names.c b/test/VFS/external-names.c
new file mode 100644 (file)
index 0000000..aa0bd67
--- /dev/null
+++ b/test/VFS/external-names.c
@@ -0,0 +1,35 @@
+// RUN: sed -e "s:INPUT_DIR:%S/Inputs:g" -e "s:OUT_DIR:%t:g" -e "s:EXTERNAL_NAMES:true:" %S/Inputs/use-external-names.yaml > %t.external.yaml
+// RUN: sed -e "s:INPUT_DIR:%S/Inputs:g" -e "s:OUT_DIR:%t:g" -e "s:EXTERNAL_NAMES:false:" %S/Inputs/use-external-names.yaml > %t.yaml
+// REQUIRES: shell
+
+#include "external-names.h"
+
+////
+// Preprocessor (__FILE__ macro and # directives):
+
+// RUN: %clang_cc1 -I %t -ivfsoverlay %t.external.yaml -E %s | FileCheck -check-prefix=CHECK-PP-EXTERNAL %s
+// CHECK-PP-EXTERNAL: # {{[0-9]*}} "[[NAME:.*Inputs.external-names.h]]"
+// CHECK-PP-EXTERNAL-NEXT: void foo(char **c) {
+// CHECK-PP-EXTERNAL-NEXT: *c = "[[NAME]]";
+
+// RUN: %clang_cc1 -I %t -ivfsoverlay %t.yaml -E %s | FileCheck -check-prefix=CHECK-PP %s
+// CHECK-PP-NOT: Inputs
+
+////
+// Diagnostics:
+
+// RUN: %clang_cc1 -I %t -ivfsoverlay %t.external.yaml -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-DIAG-EXTERNAL %s
+// CHECK-DIAG-EXTERNAL: {{.*}}Inputs{{.}}external-names.h:{{[0-9]*:[0-9]*}}: warning: incompatible pointer
+
+// RUN: %clang_cc1 -I %t -ivfsoverlay %t.yaml -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-DIAG %s
+// CHECK-DIAG-NOT: Inputs
+
+////
+// Debug info
+
+// RUN: %clang_cc1 -I %t -ivfsoverlay %t.external.yaml -triple %itanium_abi_triple -g -emit-llvm %s -o - | FileCheck -check-prefix=CHECK-DEBUG-EXTERNAL %s
+// CHECK-DEBUG-EXTERNAL: ![[Num:[0-9]*]] = metadata !{metadata !"{{.*}}Inputs{{.}}external-names.h
+// CHECK-DEBUG-EXTERNAL: metadata !{i32 {{[0-9]*}}, metadata ![[Num]]{{.*}}DW_TAG_file_type
+
+// RUN: %clang_cc1 -I %t -ivfsoverlay %t.yaml -triple %itanium_abi_triple -g -emit-llvm %s -o - | FileCheck -check-prefix=CHECK-DEBUG %s
+// CHECK-DEBUG-NOT: Inputs

diff --git a/test/VFS/module-import.m b/test/VFS/module-import.m
index 80f0ea58f1d4bc4ac175ce78cdb2b3bf44c1472e..3cfd906d7679f37b7aa1e4a67587bcaee1564a32 100644 (file)
--- a/test/VFS/module-import.m
+++ b/test/VFS/module-import.m
@@ -1,3 +1,4 @@
+// RUN: rm -rf %t
 // RUN: sed -e "s:INPUT_DIR:%S/Inputs:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
 // REQUIRES: shell

diff --git a/unittests/Basic/FileManagerTest.cpp b/unittests/Basic/FileManagerTest.cpp
index 5b35e687350f87d5aa90e7cc022cb5ab42969d6e..a9fd466a02b5d93915d2f8849d8d08c3b16451d0 100644 (file)
--- a/unittests/Basic/FileManagerTest.cpp
+++ b/unittests/Basic/FileManagerTest.cpp
@@ -28,10 +28,13 @@ private:
 
   void InjectFileOrDirectory(const char *Path, ino_t INode, bool IsFile) {
     FileData Data;
-    memset(&Data, 0, sizeof(FileData));
-    llvm::sys::fs::UniqueID ID(1, INode);
-    Data.UniqueID = ID;
+    Data.Name = Path;
+    Data.Size = 0;
+    Data.ModTime = 0;
+    Data.UniqueID = llvm::sys::fs::UniqueID(1, INode);
     Data.IsDirectory = !IsFile;
+    Data.IsNamedPipe = false;
+    Data.InPCH = false;
     StatCalls[Path] = Data;
   }