saudi created this revision. saudi added reviewers: rnk, thakis, dexonsmith, Bigcheese. saudi added a project: clang. saudi requested review of this revision. Herald added a subscriber: cfe-commits.
We noticed that the dependency collectors of `clang` may report duplicate paths under Windows, when the differences are only in casing or separators (`/` vs `\`) This problem affects the correctness of the information output by `clang-scan-deps`, which in turn, affects our integration with the `Fastbuild` cache. For separators, it can happen as clang internally computes the full path of a header from an `#include` directive using native separator. Example with argument `-I C:\includes`: `#include <subdir/header.h>` -> internally represented in the `FileManager` as `C:\includes\subdir/header.h` `#include "header.h"` from another header in `subdir` folder -> `C:\includes\subdir\header.h` The same applies with casing discrepancies that can be found sometimes in headers from external SDKs. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D102339 Files: clang/lib/Frontend/DependencyFile.cpp clang/test/Frontend/dependency-gen-windows-duplicates.c Index: clang/test/Frontend/dependency-gen-windows-duplicates.c =================================================================== --- /dev/null +++ clang/test/Frontend/dependency-gen-windows-duplicates.c @@ -0,0 +1,27 @@ +// REQUIRES: system-windows + +// RUN: rm -rf %t.dir +// RUN: mkdir -p %t.dir/subdir +// RUN: echo > %t.dir/subdir/x.h +// RUN: cp %s %t.dir/test.c +// RUN: cd %t.dir + +// RUN: %clang -MD -MF - %t.dir/test.c -fsyntax-only -I %t.dir/subdir | FileCheck --ignore-case %s +// CHECK: test.o: +// CHECK-NEXT: \test.c +// CHECK-NEXT: \subdir\x.h +// File x.h must appear only once (case insensitive check). +// CHECK-NOT: x.h + +// Include x.h several times, with different casing and separators. +// Since all paths are passed to clang as absolute, all dependencies are absolute paths. +// We expect the output dependencies to contain only one line for file x.h + +// Test case sensitivity. +#include "subdir/x.h" +#include "SubDir/X.h" + +// Test separator sensitivity: +// clang will internally concatenates x.h paths using the Windows native separator. +#include <x.h> + Index: clang/lib/Frontend/DependencyFile.cpp =================================================================== --- clang/lib/Frontend/DependencyFile.cpp +++ clang/lib/Frontend/DependencyFile.cpp @@ -141,7 +141,18 @@ } bool DependencyCollector::addDependency(StringRef Filename) { - if (Seen.insert(Filename).second) { + StringRef SearchPath; +#ifdef _WIN32 + // Make the search insensitive to case and separators. + llvm::SmallString<256> TmpPath = Filename; + std::replace(TmpPath.begin(), TmpPath.end(), '/', '\\'); + std::transform(TmpPath.begin(), TmpPath.end(), TmpPath.begin(), ::tolower); + SearchPath = TmpPath.str(); +#else + SearchPath = Filename; +#endif + + if (Seen.insert(SearchPath).second) { Dependencies.push_back(std::string(Filename)); return true; }
Index: clang/test/Frontend/dependency-gen-windows-duplicates.c =================================================================== --- /dev/null +++ clang/test/Frontend/dependency-gen-windows-duplicates.c @@ -0,0 +1,27 @@ +// REQUIRES: system-windows + +// RUN: rm -rf %t.dir +// RUN: mkdir -p %t.dir/subdir +// RUN: echo > %t.dir/subdir/x.h +// RUN: cp %s %t.dir/test.c +// RUN: cd %t.dir + +// RUN: %clang -MD -MF - %t.dir/test.c -fsyntax-only -I %t.dir/subdir | FileCheck --ignore-case %s +// CHECK: test.o: +// CHECK-NEXT: \test.c +// CHECK-NEXT: \subdir\x.h +// File x.h must appear only once (case insensitive check). +// CHECK-NOT: x.h + +// Include x.h several times, with different casing and separators. +// Since all paths are passed to clang as absolute, all dependencies are absolute paths. +// We expect the output dependencies to contain only one line for file x.h + +// Test case sensitivity. +#include "subdir/x.h" +#include "SubDir/X.h" + +// Test separator sensitivity: +// clang will internally concatenates x.h paths using the Windows native separator. +#include <x.h> + Index: clang/lib/Frontend/DependencyFile.cpp =================================================================== --- clang/lib/Frontend/DependencyFile.cpp +++ clang/lib/Frontend/DependencyFile.cpp @@ -141,7 +141,18 @@ } bool DependencyCollector::addDependency(StringRef Filename) { - if (Seen.insert(Filename).second) { + StringRef SearchPath; +#ifdef _WIN32 + // Make the search insensitive to case and separators. + llvm::SmallString<256> TmpPath = Filename; + std::replace(TmpPath.begin(), TmpPath.end(), '/', '\\'); + std::transform(TmpPath.begin(), TmpPath.end(), TmpPath.begin(), ::tolower); + SearchPath = TmpPath.str(); +#else + SearchPath = Filename; +#endif + + if (Seen.insert(SearchPath).second) { Dependencies.push_back(std::string(Filename)); return true; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits