Skip to content

Commit 9ab6d82

Browse files
committed
[clang-scan-deps] Add basic support for modules.
This fixes two issues that prevent simple uses of 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. Differential Revision: https://reviews.llvm.org/D68835
1 parent 9671d1d commit 9ab6d82

File tree

4 files changed

+99
-4
lines changed

4 files changed

+99
-4
lines changed

clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,32 @@ DependencyScanningFilesystemSharedCache::get(StringRef Key) {
122122
return It.first->getValue();
123123
}
124124

125+
/// Whitelist file extensions that should be minimized, treating no extension as
126+
/// a source file that should be minimized.
127+
///
128+
/// This is kinda hacky, it would be better if we knew what kind of file Clang
129+
/// was expecting instead.
130+
static bool shouldMinimize(StringRef Filename) {
131+
StringRef Ext = llvm::sys::path::extension(Filename);
132+
if (Ext.empty())
133+
return true; // C++ standard library
134+
return llvm::StringSwitch<bool>(Ext)
135+
.CasesLower(".c", ".cc", ".cpp", ".c++", ".cxx", true)
136+
.CasesLower(".h", ".hh", ".hpp", ".h++", ".hxx", true)
137+
.CasesLower(".m", ".mm", true)
138+
.CasesLower(".i", ".ii", ".mi", ".mmi", true)
139+
.CasesLower(".def", ".inc", true)
140+
.Default(false);
141+
}
142+
143+
144+
static bool shouldCacheStatFailures(StringRef Filename) {
145+
StringRef Ext = llvm::sys::path::extension(Filename);
146+
if (Ext.empty())
147+
return false; // This may be the module cache directory.
148+
return shouldMinimize(Filename); // Only cache stat failures on source files.
149+
}
150+
125151
llvm::ErrorOr<const CachedFileSystemEntry *>
126152
DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
127153
const StringRef Filename) {
@@ -132,7 +158,8 @@ DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
132158
// FIXME: Handle PCM/PCH files.
133159
// FIXME: Handle module map files.
134160

135-
bool KeepOriginalSource = IgnoredFiles.count(Filename);
161+
bool KeepOriginalSource = IgnoredFiles.count(Filename) ||
162+
!shouldMinimize(Filename);
136163
DependencyScanningFilesystemSharedCache::SharedFileSystemEntry
137164
&SharedCacheEntry = SharedCache.get(Filename);
138165
const CachedFileSystemEntry *Result;
@@ -143,9 +170,16 @@ DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
143170
if (!CacheEntry.isValid()) {
144171
llvm::vfs::FileSystem &FS = getUnderlyingFS();
145172
auto MaybeStatus = FS.status(Filename);
146-
if (!MaybeStatus)
147-
CacheEntry = CachedFileSystemEntry(MaybeStatus.getError());
148-
else if (MaybeStatus->isDirectory())
173+
if (!MaybeStatus) {
174+
if (!shouldCacheStatFailures(Filename))
175+
// HACK: We need to always restat non source files if the stat fails.
176+
// This is because Clang first looks up the module cache and module
177+
// files before building them, and then looks for them again. If we
178+
// cache the stat failure, it won't see them the second time.
179+
return MaybeStatus.getError();
180+
else
181+
CacheEntry = CachedFileSystemEntry(MaybeStatus.getError());
182+
} else if (MaybeStatus->isDirectory())
149183
CacheEntry = CachedFileSystemEntry::createDirectoryEntry(
150184
std::move(*MaybeStatus));
151185
else
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module header1 {
2+
header "header.h"
3+
}
4+
5+
module header2 {
6+
header "header2.h"
7+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
[
2+
{
3+
"directory": "DIR",
4+
"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 -fimplicit-modules -fimplicit-module-maps",
5+
"file": "DIR/modules_cdb_input2.cpp"
6+
},
7+
{
8+
"directory": "DIR",
9+
"command": "clang -E DIR/modules_cdb_input.cpp -IInputs -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps",
10+
"file": "DIR/modules_cdb_input.cpp"
11+
}
12+
]
13+

clang/test/ClangScanDeps/modules.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// RUN: rm -rf %t.dir
2+
// RUN: rm -rf %t.cdb
3+
// RUN: rm -rf %t.module-cache
4+
// RUN: mkdir -p %t.dir
5+
// RUN: cp %s %t.dir/modules_cdb_input.cpp
6+
// RUN: cp %s %t.dir/modules_cdb_input2.cpp
7+
// RUN: mkdir %t.dir/Inputs
8+
// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
9+
// RUN: cp %S/Inputs/header2.h %t.dir/Inputs/header2.h
10+
// RUN: cp %S/Inputs/module.modulemap %t.dir/Inputs/module.modulemap
11+
// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/modules_cdb.json > %t.cdb
12+
//
13+
// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | \
14+
// RUN: FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
15+
//
16+
// The output order is non-deterministic when using more than one thread,
17+
// so check the output using two runs. Note that the 'NOT' check is not used
18+
// as it might fail if the results for `modules_cdb_input.cpp` are reported before
19+
// `modules_cdb_input2.cpp`.
20+
//
21+
// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess-minimized-sources | \
22+
// RUN: FileCheck --check-prefix=CHECK1 %s
23+
// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess | \
24+
// RUN: FileCheck --check-prefix=CHECK1 %s
25+
// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess-minimized-sources | \
26+
// RUN: FileCheck --check-prefix=CHECK2 %s
27+
// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess | \
28+
// RUN: FileCheck --check-prefix=CHECK2 %s
29+
30+
#include "header.h"
31+
32+
// CHECK1: modules_cdb_input2.cpp
33+
// CHECK1-NEXT: modules_cdb_input2.cpp
34+
// CHECK1-NEXT: Inputs{{/|\\}}module.modulemap
35+
// CHECK1-NEXT: Inputs{{/|\\}}header2.h
36+
// CHECK1: Inputs{{/|\\}}header.h
37+
38+
// CHECK2: modules_cdb_input.cpp
39+
// CHECK2-NEXT: Inputs{{/|\\}}module.modulemap
40+
// CHECK2-NEXT: Inputs{{/|\\}}header.h
41+
// CHECK2NO-NOT: header2

0 commit comments

Comments
 (0)