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