r202539 - Reapply fixed "Honour 'use-external-names' in FileManager"
Ben Langmuir
blangmuir at apple.com
Fri Feb 28 13:16:07 PST 2014
Author: benlangmuir
Date: Fri Feb 28 15:16:07 2014
New Revision: 202539
URL: http://llvm.org/viewvc/llvm-project?rev=202539&view=rev
Log:
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.
Added:
cfe/trunk/test/VFS/Inputs/external-names.h
cfe/trunk/test/VFS/Inputs/use-external-names.yaml
cfe/trunk/test/VFS/external-names.c
Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Basic/FileSystemStatCache.h
cfe/trunk/include/clang/Basic/VirtualFileSystem.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Basic/FileSystemStatCache.cpp
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Lex/PTHLexer.cpp
cfe/trunk/test/VFS/module-import.m
cfe/trunk/unittests/Basic/FileManagerTest.cpp
Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=202539&r1=202538&r2=202539&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Fri Feb 28 15:16:07 2014
@@ -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; }
Modified: cfe/trunk/include/clang/Basic/FileSystemStatCache.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileSystemStatCache.h?rev=202539&r1=202538&r2=202539&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileSystemStatCache.h (original)
+++ cfe/trunk/include/clang/Basic/FileSystemStatCache.h Fri Feb 28 15:16:07 2014
@@ -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;
Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=202539&r1=202538&r2=202539&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Fri Feb 28 15:16:07 2014
@@ -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.
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=202539&r1=202538&r2=202539&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Fri Feb 28 15:16:07 2014
@@ -282,7 +282,7 @@ const FileEntry *FileManager::getFile(St
}
// 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;
Modified: cfe/trunk/lib/Basic/FileSystemStatCache.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileSystemStatCache.cpp?rev=202539&r1=202538&r2=202539&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileSystemStatCache.cpp (original)
+++ cfe/trunk/lib/Basic/FileSystemStatCache.cpp Fri Feb 28 15:16:07 2014
@@ -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();
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=202539&r1=202538&r2=202539&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Fri Feb 28 15:16:07 2014
@@ -83,6 +83,7 @@ error_code FileSystem::getBufferForFile(
/// \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::openFileForRe
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(cons
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(
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>
Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=202539&r1=202538&r2=202539&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri Feb 28 15:16:07 2014
@@ -886,7 +886,7 @@ static void compileModule(CompilerInstan
// 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(
Modified: cfe/trunk/lib/Lex/PTHLexer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PTHLexer.cpp?rev=202539&r1=202538&r2=202539&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PTHLexer.cpp (original)
+++ cfe/trunk/lib/Lex/PTHLexer.cpp Fri Feb 28 15:16:07 2014
@@ -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;
Added: cfe/trunk/test/VFS/Inputs/external-names.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/Inputs/external-names.h?rev=202539&view=auto
==============================================================================
--- cfe/trunk/test/VFS/Inputs/external-names.h (added)
+++ cfe/trunk/test/VFS/Inputs/external-names.h Fri Feb 28 15:16:07 2014
@@ -0,0 +1,4 @@
+void foo(char **c) {
+ *c = __FILE__;
+ int x = c; // produce a diagnostic
+}
Added: cfe/trunk/test/VFS/Inputs/use-external-names.yaml
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/Inputs/use-external-names.yaml?rev=202539&view=auto
==============================================================================
--- cfe/trunk/test/VFS/Inputs/use-external-names.yaml (added)
+++ cfe/trunk/test/VFS/Inputs/use-external-names.yaml Fri Feb 28 15:16:07 2014
@@ -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'
+ }]
+}
Added: cfe/trunk/test/VFS/external-names.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/external-names.c?rev=202539&view=auto
==============================================================================
--- cfe/trunk/test/VFS/external-names.c (added)
+++ cfe/trunk/test/VFS/external-names.c Fri Feb 28 15:16:07 2014
@@ -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
Modified: cfe/trunk/test/VFS/module-import.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/module-import.m?rev=202539&r1=202538&r2=202539&view=diff
==============================================================================
--- cfe/trunk/test/VFS/module-import.m (original)
+++ cfe/trunk/test/VFS/module-import.m Fri Feb 28 15:16:07 2014
@@ -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
Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=202539&r1=202538&r2=202539&view=diff
==============================================================================
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Fri Feb 28 15:16:07 2014
@@ -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;
}
More information about the cfe-commits
mailing list