From d066fe94b043330d26e29f3911aca79e52ebb0a9 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Fri, 28 Feb 2014 21:16:07 +0000 Subject: [PATCH] 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 --- include/clang/Basic/FileManager.h | 13 ++++---- include/clang/Basic/FileSystemStatCache.h | 2 ++ include/clang/Basic/VirtualFileSystem.h | 2 ++ lib/Basic/FileManager.cpp | 2 +- lib/Basic/FileSystemStatCache.cpp | 1 + lib/Basic/VirtualFileSystem.cpp | 37 ++++++++++++++++++----- lib/Frontend/CompilerInstance.cpp | 2 +- lib/Lex/PTHLexer.cpp | 1 + test/VFS/Inputs/external-names.h | 4 +++ test/VFS/Inputs/use-external-names.yaml | 7 +++++ test/VFS/external-names.c | 35 +++++++++++++++++++++ test/VFS/module-import.m | 1 + unittests/Basic/FileManagerTest.cpp | 9 ++++-- 13 files changed, 97 insertions(+), 19 deletions(-) create mode 100644 test/VFS/Inputs/external-names.h create mode 100644 test/VFS/Inputs/use-external-names.yaml create mode 100644 test/VFS/external-names.c diff --git a/include/clang/Basic/FileManager.h b/include/clang/Basic/FileManager.h index b5b7231e13..1e13442770 100644 --- 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 913e41e4ca..7ed0acc7a8 100644 --- 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 bd568120b2..8f144da917 100644 --- 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 f7e566b977..5784f31843 100644 --- 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 b225facbad..e50dc1b5ed 100644 --- 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 f6d88c1860..d4845e6f36 100644 --- 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 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 VFSFromYAML::status(const Twine &Path) { if (FileEntry *F = dyn_cast(*Result)) { ErrorOr 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 diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp index cdc4b64af7..a2b321ff6c 100644 --- 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 cdc5d7e338..dd8363df9c 100644 --- 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 index 0000000000..8b0baa3f02 --- /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 index 0000000000..b9ea6342cf --- /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 index 0000000000..aa0bd67453 --- /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 80f0ea58f1..3cfd906d76 100644 --- 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 5b35e68735..a9fd466a02 100644 --- 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; } -- 2.40.0