Skip to content

Commit 175e9ce

Browse files
committed
Update IsInSysroot -> IsShareable
* We need to track more than just sysroot, so start with sysroot and resource directory as a starting point, but allow the ability to capture more as they are discovered in the future. * Check a subset of the compiler invocation of module compilations for determining if a module can be shared. * When a module has a prebuilt module dependency, consider is not shareable for now.
1 parent 032d7f8 commit 175e9ce

File tree

5 files changed

+219
-42
lines changed

5 files changed

+219
-42
lines changed

clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,13 @@ struct ModuleDeps {
114114
/// Whether this is a "system" module.
115115
bool IsSystem;
116116

117-
/// Whether this module is fully composed of file & module inputs from the
118-
/// sysroot. External paths, as opposed to virtual file paths, are always used
119-
/// for computing this value.
117+
/// Whether this module is fully composed of file & module inputs from
118+
/// locations likely to stay the same across the active development and build
119+
/// cycle. For example, when all those input paths only resolve in Sysroot.
120120
///
121-
/// This attribute is useful for identifying modules that are unlikely to
122-
/// change under an active development and build cycle.
123-
bool IsInSysroot;
121+
/// External paths, as opposed to virtual file paths, are always used
122+
/// for computing this value.
123+
bool IsShareable;
124124

125125
/// The path to the modulemap file which defines this module.
126126
///
@@ -229,7 +229,7 @@ class ModuleDepCollectorPP final : public PPCallbacks {
229229
llvm::DenseSet<const Module *> &AddedModules);
230230

231231
/// Add discovered module dependency for the given module.
232-
void addClangModule(const Module *M, const ModuleID ID, ModuleDeps &MD);
232+
void addOneModuleDep(const Module *M, const ModuleID ID, ModuleDeps &MD);
233233
};
234234

235235
/// Collects modular and non-modular dependencies of the main file by attaching
@@ -331,6 +331,12 @@ void resetBenignCodeGenOptions(frontend::ActionKind ProgramAction,
331331
const LangOptions &LangOpts,
332332
CodeGenOptions &CGOpts);
333333

334+
/// Determine if \c Input can be resolved within a shared location.
335+
///
336+
/// \param Directories Paths known to be in a shared location. e.g. Sysroot.
337+
/// \param Input Path to evaluate.
338+
bool isPathInSharedDir(ArrayRef<StringRef> Directories, const StringRef Input);
339+
334340
} // end namespace dependencies
335341
} // end namespace tooling
336342
} // end namespace clang

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Lines changed: 71 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,32 @@ static void optimizeCWD(CowCompilerInvocation &BuildInvocation, StringRef CWD) {
157157
}
158158
}
159159

160+
/// Check a subset of invocation options to determine whether the current
161+
/// context can safely be considered as shareable.
162+
static bool areOptionsInSharedDir(CowCompilerInvocation &BuildInvocation,
163+
const ArrayRef<StringRef> SharedDirs) {
164+
const auto &HSOpts = BuildInvocation.getHeaderSearchOpts();
165+
if (!isPathInSharedDir(SharedDirs, HSOpts.Sysroot))
166+
return false;
167+
168+
if (!isPathInSharedDir(SharedDirs, HSOpts.ResourceDir))
169+
return false;
170+
171+
for (const auto &Entry : HSOpts.UserEntries) {
172+
if (!Entry.IgnoreSysRoot)
173+
continue;
174+
if (!isPathInSharedDir(SharedDirs, Entry.Path))
175+
return false;
176+
}
177+
178+
for (const auto &SysPrefix : HSOpts.SystemHeaderPrefixes) {
179+
if (!isPathInSharedDir(SharedDirs, SysPrefix.Prefix))
180+
return false;
181+
}
182+
183+
return true;
184+
}
185+
160186
static std::vector<std::string> splitString(std::string S, char Separator) {
161187
SmallVector<StringRef> Segments;
162188
StringRef(S).split(Segments, Separator, /*MaxSplit=*/-1, /*KeepEmpty=*/false);
@@ -212,6 +238,25 @@ void dependencies::resetBenignCodeGenOptions(frontend::ActionKind ProgramAction,
212238
}
213239
}
214240

241+
bool dependencies::isPathInSharedDir(ArrayRef<StringRef> Directories,
242+
const StringRef Input) {
243+
auto PathStartsWith = [](StringRef Prefix, StringRef Path) {
244+
auto PrefixIt = llvm::sys::path::begin(Prefix),
245+
PrefixEnd = llvm::sys::path::end(Prefix);
246+
for (auto PathIt = llvm::sys::path::begin(Path),
247+
PathEnd = llvm::sys::path::end(Path);
248+
PrefixIt != PrefixEnd && PathIt != PathEnd; ++PrefixIt, ++PathIt) {
249+
if (*PrefixIt != *PathIt)
250+
return false;
251+
}
252+
return PrefixIt == PrefixEnd;
253+
};
254+
255+
return any_of(Directories, [&](StringRef Dir) {
256+
return !Dir.empty() && PathStartsWith(Dir, Input);
257+
});
258+
}
259+
215260
static CowCompilerInvocation
216261
makeCommonInvocationForModuleBuild(CompilerInvocation CI) {
217262
CI.resetNonModularOptions();
@@ -699,13 +744,15 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
699744
MD.ID.ModuleName = M->getFullModuleName();
700745
MD.IsSystem = M->IsSystem;
701746

702-
// Start off with the assumption that this module is in the sysroot when there
747+
// Start off with the assumption that this module is shareable when there
703748
// is a sysroot provided. As more dependencies are discovered, check if those
704-
// come from the provided sysroot.
705-
const StringRef CurrSysroot = MDC.ScanInstance.getHeaderSearchOpts().Sysroot;
706-
MD.IsInSysroot =
707-
!CurrSysroot.empty() &&
708-
(llvm::sys::path::root_directory(CurrSysroot) != CurrSysroot);
749+
// come from the provided shared directories.
750+
const llvm::SmallVector<StringRef> SharedDirs = {
751+
MDC.ScanInstance.getHeaderSearchOpts().Sysroot,
752+
MDC.ScanInstance.getHeaderSearchOpts().ResourceDir};
753+
MD.IsShareable =
754+
!SharedDirs[0].empty() &&
755+
(llvm::sys::path::root_directory(SharedDirs[0]) != SharedDirs[0]);
709756

710757
// For modules which use export_as link name, the linked product that of the
711758
// corresponding export_as-named module.
@@ -748,10 +795,10 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
748795
MDC.ScanInstance.getASTReader()->visitInputFileInfos(
749796
*MF, /*IncludeSystem=*/true,
750797
[&](const serialization::InputFileInfo &IFI, bool IsSystem) {
751-
if (MD.IsInSysroot) {
798+
if (MD.IsShareable) {
752799
auto FullFilePath = ASTReader::ResolveImportedPath(
753800
PathBuf, IFI.UnresolvedImportedFilename, MF->BaseDirectory);
754-
MD.IsInSysroot = FullFilePath->starts_with(CurrSysroot);
801+
MD.IsShareable = isPathInSharedDir(SharedDirs, *FullFilePath);
755802
PathBuf.resize_for_overwrite(256);
756803
}
757804
if (!(IFI.TopLevel && IFI.ModuleMap))
@@ -795,6 +842,10 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
795842
}
796843
});
797844

845+
// Check provided input paths from the invocation for determining IsShareable.
846+
if (MD.IsShareable)
847+
MD.IsShareable = areOptionsInSharedDir(CI, SharedDirs);
848+
798849
MDC.associateWithContextHash(CI, IgnoreCWD, MD);
799850

800851
// Finish the compiler invocation. Requires dependencies and the context hash.
@@ -836,8 +887,13 @@ void ModuleDepCollectorPP::addModulePrebuiltDeps(
836887
for (const Module *Import : M->Imports)
837888
if (Import->getTopLevelModule() != M->getTopLevelModule())
838889
if (MDC.isPrebuiltModule(Import->getTopLevelModule()))
839-
if (SeenSubmodules.insert(Import->getTopLevelModule()).second)
890+
if (SeenSubmodules.insert(Import->getTopLevelModule()).second) {
840891
MD.PrebuiltModuleDeps.emplace_back(Import->getTopLevelModule());
892+
// Conservatively consider the module as not shareable,
893+
// as transitive dependencies from the prebuilt module have not been
894+
// determined.
895+
MD.IsShareable = false;
896+
}
841897
}
842898

843899
void ModuleDepCollectorPP::addAllSubmoduleDeps(
@@ -850,11 +906,11 @@ void ModuleDepCollectorPP::addAllSubmoduleDeps(
850906
});
851907
}
852908

853-
void ModuleDepCollectorPP::addClangModule(const Module *M, const ModuleID ID,
854-
ModuleDeps &MD) {
909+
void ModuleDepCollectorPP::addOneModuleDep(const Module *M, const ModuleID ID,
910+
ModuleDeps &MD) {
855911
MD.ClangModuleDeps.push_back(ID);
856-
if (MD.IsInSysroot)
857-
MD.IsInSysroot = MDC.ModularDeps[M]->IsInSysroot;
912+
if (MD.IsShareable)
913+
MD.IsShareable = MDC.ModularDeps[M]->IsShareable;
858914
}
859915

860916
void ModuleDepCollectorPP::addModuleDep(
@@ -865,7 +921,7 @@ void ModuleDepCollectorPP::addModuleDep(
865921
!MDC.isPrebuiltModule(Import)) {
866922
if (auto ImportID = handleTopLevelModule(Import->getTopLevelModule()))
867923
if (AddedModules.insert(Import->getTopLevelModule()).second)
868-
addClangModule(Import->getTopLevelModule(), *ImportID, MD);
924+
addOneModuleDep(Import->getTopLevelModule(), *ImportID, MD);
869925
}
870926
}
871927
}
@@ -889,7 +945,7 @@ void ModuleDepCollectorPP::addAffectingClangModule(
889945
!MDC.isPrebuiltModule(Affecting)) {
890946
if (auto ImportID = handleTopLevelModule(Affecting))
891947
if (AddedModules.insert(Affecting).second)
892-
addClangModule(Affecting, *ImportID, MD);
948+
addOneModuleDep(Affecting, *ImportID, MD);
893949
}
894950
}
895951
}

clang/test/ClangScanDeps/modules-in-sysroot.c renamed to clang/test/ClangScanDeps/modules-in-shared-dirs.c

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
// This test verifies modules that are entirely comprised from sysroot inputs are captured in
1+
// This test verifies modules that are entirely comprised from shared directory inputs are captured in
22
// dependency information.
33

4-
// The first compilation verifies that transitive dependencies on non-sysroot input are captured.
5-
// The second compilation verifies that external paths are resolved when a vfsoverlay is applied when considering sysroot-ness.
4+
// The first compilation verifies that transitive dependencies on local input are captured.
5+
// The second compilation verifies that external paths are resolved when a
6+
// vfsoverlay for determining is-shareable.
67

78
// REQUIRES: shell
89
// RUN: rm -rf %t
@@ -15,11 +16,11 @@
1516

1617
// CHECK: "modules": [
1718
// CHECK-NEXT: {
18-
// CHECK: "is-in-sysroot": true,
19+
// CHECK: "is-shareable": true,
1920
// CHECK: "name": "A"
2021

2122
// Verify that there are no more occurances of sysroot.
22-
// CHECK-NOT: "is-in-sysroot"
23+
// CHECK-NOT: "is-shareable"
2324

2425
// CHECK: "name": "A"
2526
// CHECK: "USE_VFS"
@@ -32,12 +33,12 @@
3233
[
3334
{
3435
"directory": "DIR",
35-
"command": "clang -c DIR/client.c -isysroot DIR/MacOSX.sdk -IDIR/MacOSX.sdk/usr/include -IDIR/BuildDir -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps",
36+
"command": "clang -c DIR/client.c -isysroot DIR/Sysroot -IDIR/Sysroot/usr/include -IDIR/BuildDir -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps",
3637
"file": "DIR/client.c"
3738
},
3839
{
3940
"directory": "DIR",
40-
"command": "clang -c DIR/client.c -isysroot DIR/MacOSX.sdk -IDIR/MacOSX.sdk/usr/include -ivfsoverlay DIR/overlay.json -DUSE_VFS -IDIR/BuildDir -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps",
41+
"command": "clang -c DIR/client.c -isysroot DIR/Sysroot -IDIR/Sysroot/usr/include -ivfsoverlay DIR/overlay.json -DUSE_VFS -IDIR/BuildDir -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps",
4142
"file": "DIR/client.c"
4243
}
4344
]
@@ -48,51 +49,51 @@
4849
"case-sensitive": "false",
4950
"roots": [
5051
{
51-
"external-contents": "DIR/local/A/A_vfs.h",
52-
"name": "DIR/MacOSX.sdk/usr/include/A/A_vfs.h",
52+
"external-contents": "DIR/SysrootButNotReally/A/A_vfs.h",
53+
"name": "DIR/Sysroot/usr/include/A/A_vfs.h",
5354
"type": "file"
5455
}
5556
]
5657
}
5758

58-
//--- MacOSX.sdk/usr/include/A/module.modulemap
59+
//--- Sysroot/usr/include/A/module.modulemap
5960
module A {
6061
umbrella "."
6162
}
6263

63-
//--- MacOSX.sdk/usr/include/A/A.h
64+
//--- Sysroot/usr/include/A/A.h
6465
#ifdef USE_VFS
6566
#include <A/A_vfs.h>
6667
#endif
6768
typedef int A_t;
6869

69-
//--- local/A/A_vfs.h
70+
//--- SysrootButNotReally/A/A_vfs.h
7071
typedef int typeFromVFS;
7172

72-
//--- MacOSX.sdk/usr/include/B/module.modulemap
73+
//--- Sysroot/usr/include/B/module.modulemap
7374
module B [system] {
7475
umbrella "."
7576
}
7677

77-
//--- MacOSX.sdk/usr/include/B/B.h
78+
//--- Sysroot/usr/include/B/B.h
7879
#include <C/C.h>
7980
typedef int B_t;
8081

81-
//--- MacOSX.sdk/usr/include/C/module.modulemap
82+
//--- Sysroot/usr/include/C/module.modulemap
8283
module C [system] {
8384
umbrella "."
8485
}
8586

86-
//--- MacOSX.sdk/usr/include/C/C.h
87+
//--- Sysroot/usr/include/C/C.h
8788
#include <D/D.h>
8889

89-
//--- MacOSX.sdk/usr/include/D/module.modulemap
90+
//--- Sysroot/usr/include/D/module.modulemap
9091
module D [system] {
9192
umbrella "."
9293
}
9394

9495
// Simulate a header that will be resolved in a local directory, from a sysroot header.
95-
//--- MacOSX.sdk/usr/include/D/D.h
96+
//--- Sysroot/usr/include/D/D.h
9697
#include <HeaderNotFoundInSDK.h>
9798

9899
//--- BuildDir/module.modulemap

0 commit comments

Comments
 (0)