<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 4, 2015 at 6:04 PM, John Thompson <span dir="ltr"><<a href="mailto:john.thompson.jtsoftware@gmail.com" target="_blank">john.thompson.jtsoftware@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span class=""><div>>Aren't we already rejecting directories in the line `if (type == sys::fs::file_type::directory_file)` above? Why check the filename?</div><div><br></div></span><div>The iterator will walk the directories. That statement just keeps the directories out of the file list. The iterator also give the relative path from the starting directory, so I look for directory nodes or file names that start with "." so I can avoid stuff that is usually private, like the .svn directory, which bit me on one of the platforms. Not perfect, but close enough, I think.</div></div></blockquote><div><br></div><div>Ah, that makes sense. Thanks for the explanation.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Thanks for looking at it.</div><div><br></div><div>-John</div><div><br></div></div><div class="gmail_extra"><div><div class="h5"><br><div class="gmail_quote">On Fri, Dec 4, 2015 at 5:08 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 4, 2015 at 2:42 PM, John Thompson via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">Author: jtsoftware<br>
Date: Fri Dec 4 16:42:18 2015<br>
New Revision: 254785<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=254785&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=254785&view=rev</a><br>
Log:<br>
Added coverage check for extensionless headers, and exclude hidden dot directoryies.<br>
<br>
Added:<br>
clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/<br>
clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h<br>
clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B<br>
Modified:<br>
clang-tools-extra/trunk/modularize/CoverageChecker.cpp<br>
clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp<br>
clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize<br>
<br>
Modified: clang-tools-extra/trunk/modularize/CoverageChecker.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/CoverageChecker.cpp?rev=254785&r1=254784&r2=254785&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/CoverageChecker.cpp?rev=254785&r1=254784&r2=254785&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/modularize/CoverageChecker.cpp (original)<br>
+++ clang-tools-extra/trunk/modularize/CoverageChecker.cpp Fri Dec 4 16:42:18 2015<br>
@@ -370,12 +370,18 @@ bool CoverageChecker::collectFileSystemH<br>
I.increment(EC)) {<br>
if (EC)<br>
return false;<br>
- std::string file(I->path());<br>
+ //std::string file(I->path());<br>
+ StringRef file(I->path());<br>
I->status(Status);<br>
sys::fs::file_type type = Status.type();<br>
// If the file is a directory, ignore the name (but still recurses).<br>
if (type == sys::fs::file_type::directory_file)<br>
continue;<br>
+ // Assume directories or files starting with '.' are private and not to<br>
+ // be considered.<br>
+ if (file.startswith(".") || (file.find("\\.") != StringRef::npos)<br>
+ || (file.find("/.") != StringRef::npos))<br>
+ continue;<br></blockquote><div><br></div><div>Aren't we already rejecting directories in the line `if (type == sys::fs::file_type::directory_file)` above? Why check the filename?</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
// If the file does not have a common header extension, ignore it.<br>
if (!ModularizeUtilities::isHeader(file))<br>
continue;<br>
<br>
Modified: clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp?rev=254785&r1=254784&r2=254785&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp?rev=254785&r1=254784&r2=254785&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp (original)<br>
+++ clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp Fri Dec 4 16:42:18 2015<br>
@@ -468,7 +468,7 @@ std::string ModularizeUtilities::getCano<br>
bool ModularizeUtilities::isHeader(StringRef FileName) {<br>
StringRef Extension = llvm::sys::path::extension(FileName);<br>
if (Extension.size() == 0)<br>
- return false;<br>
+ return true;<br>
if (Extension.equals_lower(".h"))<br>
return true;<br>
if (Extension.equals_lower(".inc"))<br>
<br>
Added: clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h?rev=254785&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h?rev=254785&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h (added)<br>
+++ clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h Fri Dec 4 16:42:18 2015<br>
@@ -0,0 +1,3 @@<br>
+#error DontFindMe.h shouldn't be found.<br>
+<br>
+<br>
<br>
Added: clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B?rev=254785&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B?rev=254785&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B (added)<br>
+++ clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B Fri Dec 4 16:42:18 2015<br>
@@ -0,0 +1 @@<br>
+#define MACRO_3B 1<br>
<br>
Modified: clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize?rev=254785&r1=254784&r2=254785&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize?rev=254785&r1=254784&r2=254785&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize (original)<br>
+++ clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize Fri Dec 4 16:42:18 2015<br>
@@ -1,4 +1,5 @@<br>
# RUN: not modularize %S/Inputs/CoverageProblems/module.modulemap 2>&1 | FileCheck %s<br>
<br>
# CHECK: warning: {{.*}}{{[/\\]}}Inputs/CoverageProblems/module.modulemap does not account for file: {{.*}}{{[/\\]}}Inputs/CoverageProblems/Level3A.h<br>
+# CHECK-NEXT: warning: {{.*}}{{[/\\]}}Inputs/CoverageProblems/module.modulemap does not account for file: {{.*}}{{[/\\]}}Inputs/CoverageProblems/Level3B<br>
# CHECK-NEXT: warning: {{.*}}{{[/\\]}}Inputs/CoverageProblems/module.modulemap does not account for file: {{.*}}{{[/\\]}}Inputs/CoverageProblems/Sub/Level3B.h<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>
</blockquote></div><br><br clear="all"><br></div></div><span class="HOEnZb"><font color="#888888">-- <br><div>John Thompson<br><a href="mailto:John.Thompson.JTSoftware@gmail.com" target="_blank">John.Thompson.JTSoftware@gmail.com</a><br></div>
</font></span></div>
</blockquote></div><br></div></div>