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
diff --git a/clang/include/clang/Basic/VirtualFileSystem.h b/clang/include/clang/Basic/VirtualFileSystem.h
index 1c98f67..bd56812 100644
--- a/clang/include/clang/Basic/VirtualFileSystem.h
+++ b/clang/include/clang/Basic/VirtualFileSystem.h
@@ -29,7 +29,6 @@
/// \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 @@
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 539cbb7..f6d88c18 100644
--- a/clang/lib/Basic/VirtualFileSystem.cpp
+++ b/clang/lib/Basic/VirtualFileSystem.cpp
@@ -35,8 +35,8 @@
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 @@
return EC;
Status Result(RealStatus);
Result.setName(Path.str());
- Result.setExternalName(Path.str());
return Result;
}
@@ -253,12 +252,22 @@
};
class FileEntry : public Entry {
- std::string ExternalContentsPath;
-
public:
- FileEntry(StringRef Name, StringRef ExternalContentsPath)
- : Entry(EK_File, Name), ExternalContentsPath(ExternalContentsPath) {}
+ enum NameKind {
+ NK_NotSet,
+ NK_External,
+ NK_Virtual
+ };
+private:
+ std::string ExternalContentsPath;
+ NameKind UseName;
+public:
+ 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 @@
///
/// 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 @@
/// {
/// 'type': 'file',
/// 'name': <string>,
+/// 'use-external-name': <boolean> # Optional
/// 'external-contents': <path to external file>)
/// }
/// \endverbatim
@@ -324,14 +335,18 @@
/// \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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
KeyStatusPair Fields[] = {
KeyStatusPair("version", true),
KeyStatusPair("case-sensitive", false),
+ KeyStatusPair("use-external-names", false),
KeyStatusPair("roots", true),
};
@@ -635,6 +665,9 @@
} 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 @@
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 334b1a4..367e794 100644
--- a/clang/unittests/Basic/VirtualFileSystemTest.cpp
+++ b/clang/unittests/Basic/VirtualFileSystemTest.cpp
@@ -290,8 +290,7 @@
// 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 @@
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");