<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Feb 26, 2014 at 4:25 PM, Ben Langmuir <span dir="ltr"><<a href="mailto:blangmuir@apple.com" target="_blank">blangmuir@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: benlangmuir<br>
Date: Wed Feb 26 18:25:12 2014<br>
New Revision: 202329<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=202329&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=202329&view=rev</a><br>
Log:<br>
Add a 'use-external-names' option to VFS overlay files<br>
<br>
When true, sets the name of the file to be the name from<br>
'external-contents'. Otherwise, you get the virtual path that the file<br>
was looked up by. This will not affect any non-virtual paths, or fully<br>
virtual paths (for which there is no reasonable 'external' name anyway).<br>
<br>
The setting is available globally, but can be overriden on a per-file<br>
basis.<br>
<br>
The goal is that this setting will control which path you see in debug<br>
info, diagnostics, etc. which are sensitive to which path is used. That<br>
will come in future patches that pass the name through to FileManager.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/VirtualFileSystem.h<br>
cfe/trunk/lib/Basic/VirtualFileSystem.cpp<br>
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=202329&r1=202328&r2=202329&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=202329&r1=202328&r2=202329&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)<br>
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Wed Feb 26 18:25:12 2014<br>
@@ -29,7 +29,6 @@ namespace vfs {<br>
/// \brief The result of a \p status operation.<br>
class Status {<br>
std::string Name;<br>
- std::string ExternalName;<br>
llvm::sys::fs::UniqueID UID;<br>
llvm::sys::TimeValue MTime;<br>
uint32_t User;<br>
@@ -46,16 +45,9 @@ public:<br>
uint64_t Size, llvm::sys::fs::file_type Type,<br>
llvm::sys::fs::perms Perms);<br>
<br>
- /// \brief Returns the name this status was looked up by.<br>
+ /// \brief Returns the name that should be used for this file or directory.<br>
StringRef getName() const { return Name; }<br>
-<br>
- /// \brief Returns the name to use outside the compiler.<br>
- ///<br>
- /// For example, in diagnostics or debug info we should use this name.<br>
- StringRef getExternalName() const { return ExternalName; }<br>
-<br>
void setName(StringRef N) { Name = N; }<br>
- void setExternalName(StringRef N) { ExternalName = N; }<br>
<br>
/// @name Status interface from llvm::sys::fs<br>
/// @{<br>
<br>
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=202329&r1=202328&r2=202329&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=202329&r1=202328&r2=202329&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)<br>
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Wed Feb 26 18:25:12 2014<br>
@@ -35,8 +35,8 @@ Status::Status(const file_status &Status<br>
Status::Status(StringRef Name, StringRef ExternalName, UniqueID UID,<br>
sys::TimeValue MTime, uint32_t User, uint32_t Group,<br>
uint64_t Size, file_type Type, perms Perms)<br>
- : Name(Name), ExternalName(ExternalName), UID(UID), MTime(MTime),<br>
- User(User), Group(Group), Size(Size), Type(Type), Perms(Perms) {}<br>
+ : Name(Name), UID(UID), MTime(MTime), User(User), Group(Group), Size(Size),<br>
+ Type(Type), Perms(Perms) {}<br>
<br>
bool Status::equivalent(const Status &Other) const {<br>
return getUniqueID() == Other.getUniqueID();<br>
@@ -145,7 +145,6 @@ ErrorOr<Status> RealFileSystem::status(c<br>
return EC;<br>
Status Result(RealStatus);<br>
Result.setName(Path.str());<br>
- Result.setExternalName(Path.str());<br>
return Result;<br>
}<br>
<br>
@@ -253,12 +252,22 @@ public:<br>
};<br>
<br>
class FileEntry : public Entry {<br>
+public:<br>
+ enum NameKind {<br>
+ NK_NotSet,<br>
+ NK_External,<br>
+ NK_Virtual<br>
+ };<br>
+private:<br>
std::string ExternalContentsPath;<br>
-<br>
+ NameKind UseName;<br>
public:<br>
- FileEntry(StringRef Name, StringRef ExternalContentsPath)<br>
- : Entry(EK_File, Name), ExternalContentsPath(ExternalContentsPath) {}<br>
+ FileEntry(StringRef Name, StringRef ExternalContentsPath, NameKind UseName)<br>
+ : Entry(EK_File, Name), ExternalContentsPath(ExternalContentsPath),<br>
+ UseName(UseName) {}<br>
StringRef getExternalContentsPath() const { return ExternalContentsPath; }<br>
+ /// \brief whether to use the external path as the name for this file.<br>
+ NameKind useName() const { return UseName; }<br>
static bool classof(const Entry *E) { return E->getKind() == EK_File; }<br>
};<br>
<br>
@@ -280,6 +289,7 @@ public:<br>
///<br>
/// All configuration options are optional.<br>
/// 'case-sensitive': <boolean, default=true><br>
+/// 'use-external-names': <boolean, default=true><br>
///<br>
/// Virtual directories are represented as<br>
/// \verbatim<br>
@@ -304,6 +314,7 @@ public:<br>
/// {<br>
/// 'type': 'file',<br>
/// 'name': <string>,<br>
+/// 'use-external-name': <boolean> # Optional<br>
/// 'external-contents': <path to external file>)<br>
/// }<br>
/// \endverbatim<br>
@@ -324,14 +335,18 @@ class VFSFromYAML : public vfs::FileSyst<br>
/// \brief Whether to perform case-sensitive comparisons.<br>
///<br>
/// Currently, case-insensitive matching only works correctly with ASCII.<br>
- bool CaseSensitive; ///< Whether to perform case-sensitive comparisons.<br>
+ bool CaseSensitive;<br>
+<br>
+ /// \brief Whether to use to use the value of 'external-contents' for the<br>
+ /// names of files. This global value is overridable on a per-file basis.<br>
+ bool UseExternalNames;<br>
/// @}<br>
<br>
friend class VFSFromYAMLParser;<br>
<br>
private:<br>
VFSFromYAML(IntrusiveRefCntPtr<FileSystem> ExternalFS)<br>
- : ExternalFS(ExternalFS), CaseSensitive(true) {}<br>
+ : ExternalFS(ExternalFS), CaseSensitive(true), UseExternalNames(true) {}<br>
<br>
/// \brief Looks up \p Path in \c Roots.<br>
ErrorOr<Entry *> lookupPath(const Twine &Path);<br>
@@ -446,7 +461,8 @@ class VFSFromYAMLParser {<br>
KeyStatusPair("name", true),<br>
KeyStatusPair("type", true),<br>
KeyStatusPair("contents", false),<br>
- KeyStatusPair("external-contents", false)<br>
+ KeyStatusPair("external-contents", false),<br>
+ KeyStatusPair("use-external-name", false),<br>
};<br>
<br>
DenseMap<StringRef, KeyStatus> Keys(<br>
@@ -456,6 +472,7 @@ class VFSFromYAMLParser {<br>
std::vector<Entry *> EntryArrayContents;<br>
std::string ExternalContentsPath;<br>
std::string Name;<br>
+ FileEntry::NameKind UseExternalName = FileEntry::NK_NotSet;<br>
EntryKind Kind;<br>
<br>
for (yaml::MappingNode::iterator I = M->begin(), E = M->end(); I != E;<br>
@@ -519,6 +536,11 @@ class VFSFromYAMLParser {<br>
if (!parseScalarString(I->getValue(), Value, Buffer))<br>
return NULL;<br>
ExternalContentsPath = Value;<br>
+ } else if (Key == "use-external-name") {<br>
+ bool Val;<br>
+ if (!parseScalarBool(I->getValue(), Val))<br>
+ return NULL;<br>
+ UseExternalName = Val ? FileEntry::NK_External : FileEntry::NK_Virtual;<br>
} else {<br>
llvm_unreachable("key missing from Keys");<br>
}<br>
@@ -535,6 +557,12 @@ class VFSFromYAMLParser {<br>
if (!checkMissingKeys(N, Keys))<br>
return NULL;<br>
<br>
+ // check invalid configuration<br>
+ if (Kind == EK_Directory && UseExternalName != FileEntry::NK_NotSet) {<br></blockquote><div><br></div><div>[Found by Coverity]</div><div><br></div><div>If the YAML entry has a name field and a contents field but no kind field, we can reach this line with Kind uninitialized, resulting in UB.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ error(N, "'use-external-name' is not supported for directories");<br>
+ return NULL;<br>
+ }<br>
+<br>
// Remove trailing slash(es)<br>
StringRef Trimmed(Name);<br>
while (Trimmed.size() > 1 && sys::path::is_separator(Trimmed.back()))<br>
@@ -545,7 +573,8 @@ class VFSFromYAMLParser {<br>
Entry *Result = 0;<br>
switch (Kind) {<br>
case EK_File:<br>
- Result = new FileEntry(LastComponent, llvm_move(ExternalContentsPath));<br>
+ Result = new FileEntry(LastComponent, llvm_move(ExternalContentsPath),<br>
+ UseExternalName);<br>
break;<br>
case EK_Directory:<br>
Result = new DirectoryEntry(LastComponent, llvm_move(EntryArrayContents),<br>
@@ -583,6 +612,7 @@ public:<br>
KeyStatusPair Fields[] = {<br>
KeyStatusPair("version", true),<br>
KeyStatusPair("case-sensitive", false),<br>
+ KeyStatusPair("use-external-names", false),<br>
KeyStatusPair("roots", true),<br>
};<br>
<br>
@@ -635,6 +665,9 @@ public:<br>
} else if (Key == "case-sensitive") {<br>
if (!parseScalarBool(I->getValue(), FS->CaseSensitive))<br>
return false;<br>
+ } else if (Key == "use-external-names") {<br>
+ if (!parseScalarBool(I->getValue(), FS->UseExternalNames))<br>
+ return false;<br>
} else {<br>
llvm_unreachable("key missing from Keys");<br>
}<br>
@@ -736,17 +769,15 @@ ErrorOr<Status> VFSFromYAML::status(cons<br>
std::string PathStr(Path.str());<br>
if (FileEntry *F = dyn_cast<FileEntry>(*Result)) {<br>
ErrorOr<Status> S = ExternalFS->status(F->getExternalContentsPath());<br>
- if (S) {<br>
- assert(S->getName() == S->getExternalName() &&<br>
- S->getName() == F->getExternalContentsPath());<br>
+ assert(!S || S->getName() == F->getExternalContentsPath());<br>
+ if (S && (F->useName() == FileEntry::NK_Virtual ||<br>
+ (F->useName() == FileEntry::NK_NotSet && !UseExternalNames)))<br>
S->setName(PathStr);<br>
- }<br>
return S;<br>
} else { // directory<br>
DirectoryEntry *DE = cast<DirectoryEntry>(*Result);<br>
Status S = DE->getStatus();<br>
S.setName(PathStr);<br>
- S.setExternalName(PathStr);<br>
return S;<br>
}<br>
}<br>
<br>
Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=202329&r1=202328&r2=202329&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=202329&r1=202328&r2=202329&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)<br>
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Wed Feb 26 18:25:12 2014<br>
@@ -290,8 +290,7 @@ TEST_F(VFSFromYAMLTest, MappedFiles) {<br>
// file<br>
ErrorOr<vfs::Status> S = O->status("/file1");<br>
ASSERT_EQ(errc::success, S.getError());<br>
- EXPECT_EQ("/file1", S->getName());<br>
- EXPECT_EQ("/foo/bar/a", S->getExternalName());<br>
+ EXPECT_EQ("/foo/bar/a", S->getName());<br>
<br>
ErrorOr<vfs::Status> SLower = O->status("/foo/bar/a");<br>
EXPECT_EQ("/foo/bar/a", SLower->getName());<br>
@@ -467,6 +466,57 @@ TEST_F(VFSFromYAMLTest, IllegalVFSFile)<br>
EXPECT_EQ(24, NumDiagnostics);<br>
}<br>
<br>
+TEST_F(VFSFromYAMLTest, UseExternalName) {<br>
+ IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());<br>
+ Lower->addRegularFile("/external/file");<br>
+<br>
+ IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(<br>
+ "{ 'roots': [\n"<br>
+ " { 'type': 'file', 'name': '/A',\n"<br>
+ " 'external-contents': '/external/file'\n"<br>
+ " },\n"<br>
+ " { 'type': 'file', 'name': '/B',\n"<br>
+ " 'use-external-name': true,\n"<br>
+ " 'external-contents': '/external/file'\n"<br>
+ " },\n"<br>
+ " { 'type': 'file', 'name': '/C',\n"<br>
+ " 'use-external-name': false,\n"<br>
+ " 'external-contents': '/external/file'\n"<br>
+ " }\n"<br>
+ "] }", Lower);<br>
+ ASSERT_TRUE(NULL != FS.getPtr());<br>
+<br>
+ // default true<br>
+ EXPECT_EQ("/external/file", FS->status("/A")->getName());<br>
+ // explicit<br>
+ EXPECT_EQ("/external/file", FS->status("/B")->getName());<br>
+ EXPECT_EQ("/C", FS->status("/C")->getName());<br>
+<br>
+ // global configuration<br>
+ FS = getFromYAMLString(<br>
+ "{ 'use-external-names': false,\n"<br>
+ " 'roots': [\n"<br>
+ " { 'type': 'file', 'name': '/A',\n"<br>
+ " 'external-contents': '/external/file'\n"<br>
+ " },\n"<br>
+ " { 'type': 'file', 'name': '/B',\n"<br>
+ " 'use-external-name': true,\n"<br>
+ " 'external-contents': '/external/file'\n"<br>
+ " },\n"<br>
+ " { 'type': 'file', 'name': '/C',\n"<br>
+ " 'use-external-name': false,\n"<br>
+ " 'external-contents': '/external/file'\n"<br>
+ " }\n"<br>
+ "] }", Lower);<br>
+ ASSERT_TRUE(NULL != FS.getPtr());<br>
+<br>
+ // default<br>
+ EXPECT_EQ("/A", FS->status("/A")->getName());<br>
+ // explicit<br>
+ EXPECT_EQ("/external/file", FS->status("/B")->getName());<br>
+ EXPECT_EQ("/C", FS->status("/C")->getName());<br>
+}<br>
+<br>
TEST_F(VFSFromYAMLTest, MultiComponentPath) {<br>
IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());<br>
Lower->addRegularFile("/other");<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>