Hi John,
> + if (file.startswith(".") || (file.find("\\.") != StringRef::npos)
> + || (file.find("/.") != StringRef::npos))
This will also filter out things like ./foo.h, which probably isn't what we want
and breaks on relative/non-absolute paths.
For example, given a trivial test:
//----- test.modulemap
module foo {
header "foo.h"
export *
}
//-----------------
And some headers:
// ----- foo.h
typedef unsigned long ulong_t;
// ----- bar
typedef unsigned long ulong_t;
$ modularize -coverage-check-only test.modulemap
warning: No headers found in include path: ""
It is only OK if I specific the full path:
$ modularize -coverage-check-only /tmp/mod_test/test.modulemap
warning: /tmp/mod_test/test.modulemap does not account for file:
/tmp/mod_test/bar
A simple refinement to your StringRef checks can catch the "./" case,
but I worry about other corner cases.
Once all corner cases are identified, it may be worth considering
abstracting the logic for identifying hidden/dot directories into some common
API.
Thanks,
- Josh
At 1449268938 seconds past the Epoch, John Thompson via cfe-commits wrote:
> Author: jtsoftware
> Date: Fri Dec 4 16:42:18 2015
> New Revision: 254785
>
> URL: http://llvm.org/viewvc/llvm-project?rev=254785&view=rev
> Log:
> Added coverage check for extensionless headers, and exclude hidden dot
> directoryies.
>
> Added:
>
> clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/
>
> clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h
> clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B
> Modified:
> clang-tools-extra/trunk/modularize/CoverageChecker.cpp
> clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp
> clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize
>
> Modified: clang-tools-extra/trunk/modularize/CoverageChecker.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/CoverageChecker.cpp?rev=254785&r1=254784&r2=254785&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/modularize/CoverageChecker.cpp (original)
> +++ clang-tools-extra/trunk/modularize/CoverageChecker.cpp Fri Dec 4
> 16:42:18 2015
> @@ -370,12 +370,18 @@ bool CoverageChecker::collectFileSystemH
> I.increment(EC)) {
> if (EC)
> return false;
> - std::string file(I->path());
> + //std::string file(I->path());
> + StringRef file(I->path());
> I->status(Status);
> sys::fs::file_type type = Status.type();
> // If the file is a directory, ignore the name (but still recurses).
> if (type == sys::fs::file_type::directory_file)
> continue;
> + // Assume directories or files starting with '.' are private and not to
> + // be considered.
> + if (file.startswith(".") || (file.find("\\.") != StringRef::npos)
> + || (file.find("/.") != StringRef::npos))
> + continue;
> // If the file does not have a common header extension, ignore it.
> if (!ModularizeUtilities::isHeader(file))
> continue;
>
> Modified: clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp?rev=254785&r1=254784&r2=254785&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp (original)
> +++ clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp Fri Dec 4
> 16:42:18 2015
> @@ -468,7 +468,7 @@ std::string ModularizeUtilities::getCano
> bool ModularizeUtilities::isHeader(StringRef FileName) {
> StringRef Extension = llvm::sys::path::extension(FileName);
> if (Extension.size() == 0)
> - return false;
> + return true;
> if (Extension.equals_lower(".h"))
> return true;
> if (Extension.equals_lower(".inc"))
>
> Added:
> clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h?rev=254785&view=auto
> ==============================================================================
> ---
> clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h
> (added)
> +++
> clang-tools-extra/trunk/test/modularize/Inputs/CoverageNoProblems/Includes1/.hidden/DontFindMe.h
> Fri Dec 4 16:42:18 2015
> @@ -0,0 +1,3 @@
> +#error DontFindMe.h shouldn't be found.
> +
> +
>
> Added: clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B?rev=254785&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B
> (added)
> +++ clang-tools-extra/trunk/test/modularize/Inputs/CoverageProblems/Level3B
> Fri Dec 4 16:42:18 2015
> @@ -0,0 +1 @@
> +#define MACRO_3B 1
>
> Modified: clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize?rev=254785&r1=254784&r2=254785&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize
> (original)
> +++ clang-tools-extra/trunk/test/modularize/ProblemsCoverage.modularize Fri
> Dec 4 16:42:18 2015
> @@ -1,4 +1,5 @@
> # RUN: not modularize %S/Inputs/CoverageProblems/module.modulemap 2>&1 |
> FileCheck %s
>
> # CHECK: warning: {{.*}}{{[/\\]}}Inputs/CoverageProblems/module.modulemap
> does not account for file: {{.*}}{{[/\\]}}Inputs/CoverageProblems/Level3A.h
> +# CHECK-NEXT: warning:
> {{.*}}{{[/\\]}}Inputs/CoverageProblems/module.modulemap does not account for
> file: {{.*}}{{[/\\]}}Inputs/CoverageProblems/Level3B
> # CHECK-NEXT: warning:
> {{.*}}{{[/\\]}}Inputs/CoverageProblems/module.modulemap does not account for
> file: {{.*}}{{[/\\]}}Inputs/CoverageProblems/Sub/Level3B.h
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
