Bigcheese created this revision.
Bigcheese added a reviewer: arphaman.
Herald added subscribers: tschuett, dexonsmith.
Herald added a project: clang.
This fixes two issues that prevent simple uses of Clang modules from working.
- We would previously minimize _every_ file opened by clang, even module maps
and module pcm files. Now we only minimize files with known extensions. It
would be better if we knew which files clang intended to open as a source file,
but this works for now.
- We previously cached every lookup, even failed lookups. This is a problem
because clang stats the module cache directory before building a module and
creating that directory. If we cache that failure then the subsequent pcm load
doesn't see the module cache and fails.
Overall this still leaves us building minmized modules on disk during scanning.
This will need to be improved eventually for performance, but this is correct,
and works for now.
Repository:
rC Clang
https://reviews.llvm.org/D68835
Files:
lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
test/ClangScanDeps/Inputs/module.modulemap
test/ClangScanDeps/Inputs/modules_cdb.json
test/ClangScanDeps/modules.cpp
Index: test/ClangScanDeps/modules.cpp
===================================================================
--- /dev/null
+++ test/ClangScanDeps/modules.cpp
@@ -0,0 +1,41 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: rm -rf %t.module-cache
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/modules_cdb_input.cpp
+// RUN: cp %s %t.dir/modules_cdb_input2.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: cp %S/Inputs/header2.h %t.dir/Inputs/header2.h
+// RUN: cp %S/Inputs/module.modulemap %t.dir/Inputs/module.modulemap
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/modules_cdb.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | \
+// RUN: FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+//
+// The output order is non-deterministic when using more than one thread,
+// so check the output using two runs. Note that the 'NOT' check is not used
+// as it might fail if the results for `modules_cdb_input.cpp` are reported before
+// `modules_cdb_input2.cpp`.
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess-minimized-sources | \
+// RUN: FileCheck --check-prefix=CHECK1 %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess | \
+// RUN: FileCheck --check-prefix=CHECK1 %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess-minimized-sources | \
+// RUN: FileCheck --check-prefix=CHECK2 %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess | \
+// RUN: FileCheck --check-prefix=CHECK2 %s
+
+#include "header.h"
+
+// CHECK1: modules_cdb_input2.cpp
+// CHECK1-NEXT: modules_cdb_input2.cpp
+// CHECK1-NEXT: Inputs{{/|\\}}module.modulemap
+// CHECK1-NEXT: Inputs{{/|\\}}header2.h
+// CHECK1: Inputs{{/|\\}}header.h
+
+// CHECK2: modules_cdb_input.cpp
+// CHECK2-NEXT: Inputs/module.modulemap
+// CHECK2-NEXT: Inputs{{/|\\}}header.h
+// CHECK2NO-NOT: header2
Index: test/ClangScanDeps/Inputs/modules_cdb.json
===================================================================
--- /dev/null
+++ test/ClangScanDeps/Inputs/modules_cdb.json
@@ -0,0 +1,13 @@
+[
+{
+ "directory": "DIR",
+ "command": "clang -E -fsyntax-only DIR/modules_cdb_input2.cpp -IInputs -D INCLUDE_HEADER2 -MD -MF DIR/modules_cdb2.d -fmodules -fmodules-cache-path=DIR/module-cache",
+ "file": "DIR/modules_cdb_input2.cpp"
+},
+{
+ "directory": "DIR",
+ "command": "clang -E DIR/modules_cdb_input.cpp -IInputs -fmodules -fmodules-cache-path=DIR/module-cache",
+ "file": "DIR/modules_cdb_input.cpp"
+}
+]
+
Index: test/ClangScanDeps/Inputs/module.modulemap
===================================================================
--- /dev/null
+++ test/ClangScanDeps/Inputs/module.modulemap
@@ -0,0 +1,7 @@
+module header1 {
+ header "header.h"
+}
+
+module header2 {
+ header "header2.h"
+}
Index: lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===================================================================
--- lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -122,6 +122,31 @@
return It.first->getValue();
}
+/// Whitelist file extensions that should be minimized, treating no extension as
+/// a source file that should be minimized.
+///
+/// This is kinda hacky, it would be better if we knew what kind of file Clang
+/// was expecting instead.
+static bool shouldMinimize(StringRef Filename) {
+ StringRef Ext = llvm::sys::path::extension(Filename);
+ if (Ext.empty())
+ return true; // C++ standard library
+ return llvm::StringSwitch<bool>(Ext)
+ .CasesLower(".c", ".cc", ".cpp", ".c++", ".cxx", true)
+ .CasesLower(".h", ".hh", ".hpp", ".h++", ".hxx", true)
+ .CasesLower(".m", ".mm", true)
+ .CasesLower(".def", ".inc", true)
+ .Default(false);
+}
+
+
+static bool shouldCacheStatFailures(StringRef Filename) {
+ StringRef Ext = llvm::sys::path::extension(Filename);
+ if (Ext.empty())
+ return false; // This may be the module cache directory.
+ return shouldMinimize(Filename); // Only cache stat failures on source files.
+}
+
llvm::ErrorOr<const CachedFileSystemEntry *>
DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
const StringRef Filename) {
@@ -132,7 +157,8 @@
// FIXME: Handle PCM/PCH files.
// FIXME: Handle module map files.
- bool KeepOriginalSource = IgnoredFiles.count(Filename);
+ bool KeepOriginalSource = IgnoredFiles.count(Filename) ||
+ !shouldMinimize(Filename);
DependencyScanningFilesystemSharedCache::SharedFileSystemEntry
&SharedCacheEntry = SharedCache.get(Filename);
const CachedFileSystemEntry *Result;
@@ -143,9 +169,16 @@
if (!CacheEntry.isValid()) {
llvm::vfs::FileSystem &FS = getUnderlyingFS();
auto MaybeStatus = FS.status(Filename);
- if (!MaybeStatus)
- CacheEntry = CachedFileSystemEntry(MaybeStatus.getError());
- else if (MaybeStatus->isDirectory())
+ if (!MaybeStatus) {
+ if (!shouldCacheStatFailures(Filename))
+ // HACK: We need to always restat non source files if the stat fails.
+ // This is because Clang first looks up the module cache and module
+ // files before building them, and then looks for them again. If we
+ // cache the stat failure, it won't see them the second time.
+ return MaybeStatus.getError();
+ else
+ CacheEntry = CachedFileSystemEntry(MaybeStatus.getError());
+ } else if (MaybeStatus->isDirectory())
CacheEntry = CachedFileSystemEntry::createDirectoryEntry(
std::move(*MaybeStatus));
else
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
