Skip to content

Commit cbd5ba2

Browse files
vsapsaijansvoboda11
vsapsai
and
jansvoboda11
authored
[Modules] Don't search for modulemaps in the immediate sub-directories of search paths for recent Apple SDKs. (#100005)
Such searches can be costly and non-intuitive. We've seen complaints from developers that they don't expect clang to find modules on their own and not in search paths that developers provide. Keeping the search of modulemaps in subdirectories for code completion as it provides better user experience. If you are defining module "UsefulCode" in "include/UnrelatedName/module.modulemap", it is recommended to rename the directory "UnrelatedName" to "UsefulCode". If you cannot do so, you can add to "include/module.modulemap" a line like `extern module UsefulCode "UnrelatedName/module.modulemap"`, so clang can find module "UsefulCode" without checking each subdirectory in "include/". rdar://106677321 --------- Co-authored-by: Jan Svoboda <[email protected]>
1 parent d0c8e26 commit cbd5ba2

File tree

7 files changed

+103
-13
lines changed

7 files changed

+103
-13
lines changed

‎clang/include/clang/Driver/Options.td‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3232,6 +3232,10 @@ def fimplicit_module_maps : Flag <["-"], "fimplicit-module-maps">, Group<f_Group
32323232
Visibility<[ClangOption, CC1Option, CLOption]>,
32333233
HelpText<"Implicitly search the file system for module map files.">,
32343234
MarshallingInfoFlag<HeaderSearchOpts<"ImplicitModuleMaps">>;
3235+
defm modulemap_allow_subdirectory_search : BoolFOption <"modulemap-allow-subdirectory-search",
3236+
HeaderSearchOpts<"AllowModuleMapSubdirectorySearch">, DefaultTrue,
3237+
PosFlag<SetTrue, [], [], "Allow to search for module maps in subdirectories of search paths">,
3238+
NegFlag<SetFalse>, BothFlags<[NoXarchOption], [ClangOption, CC1Option]>>;
32353239
defm modules : BoolFOption<"modules",
32363240
LangOpts<"Modules">, Default<fcxx_modules.KeyPath>,
32373241
PosFlag<SetTrue, [], [ClangOption, CC1Option],

‎clang/include/clang/Lex/HeaderSearchOptions.h‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,12 @@ class HeaderSearchOptions {
270270
LLVM_PREFERRED_TYPE(bool)
271271
unsigned ModulesIncludeVFSUsage : 1;
272272

273+
/// Whether we should look for a module in module maps only in provided
274+
/// header search paths or if we are allowed to look for module maps in
275+
/// subdirectories of provided paths too.
276+
LLVM_PREFERRED_TYPE(bool)
277+
unsigned AllowModuleMapSubdirectorySearch : 1;
278+
273279
HeaderSearchOptions(StringRef _Sysroot = "/")
274280
: Sysroot(_Sysroot), ModuleFormat("raw"), DisableModuleHash(false),
275281
ImplicitModuleMaps(false), ModuleMapFileHomeIsCwd(false),
@@ -285,7 +291,8 @@ class HeaderSearchOptions {
285291
ModulesSkipHeaderSearchPaths(false),
286292
ModulesSkipPragmaDiagnosticMappings(false),
287293
ModulesPruneNonAffectingModuleMaps(true), ModulesHashContent(false),
288-
ModulesStrictContextHash(false), ModulesIncludeVFSUsage(false) {}
294+
ModulesStrictContextHash(false), ModulesIncludeVFSUsage(false),
295+
AllowModuleMapSubdirectorySearch(true) {}
289296

290297
/// AddPath - Add the \p Path path to the specified \p Group list.
291298
void AddPath(StringRef Path, frontend::IncludeDirGroup Group,

‎clang/lib/Driver/ToolChains/Clang.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3960,6 +3960,9 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
39603960
options::OPT_fno_modules_strict_decluse, false))
39613961
CmdArgs.push_back("-fmodules-strict-decluse");
39623962

3963+
Args.addOptOutFlag(CmdArgs, options::OPT_fmodulemap_allow_subdirectory_search,
3964+
options::OPT_fno_modulemap_allow_subdirectory_search);
3965+
39633966
// -fno-implicit-modules turns off implicitly compiling modules on demand.
39643967
bool ImplicitModules = false;
39653968
if (!Args.hasFlag(options::OPT_fimplicit_modules,

‎clang/lib/Driver/ToolChains/Darwin.cpp‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3036,6 +3036,35 @@ void Darwin::addClangTargetOptions(
30363036
if (!DriverArgs.hasArgNoClaim(options::OPT_fdefine_target_os_macros,
30373037
options::OPT_fno_define_target_os_macros))
30383038
CC1Args.push_back("-fdefine-target-os-macros");
3039+
3040+
// Disable subdirectory modulemap search on sufficiently recent SDKs.
3041+
if (SDKInfo &&
3042+
!DriverArgs.hasFlag(options::OPT_fmodulemap_allow_subdirectory_search,
3043+
options::OPT_fno_modulemap_allow_subdirectory_search,
3044+
false)) {
3045+
bool RequiresSubdirectorySearch;
3046+
VersionTuple SDKVersion = SDKInfo->getVersion();
3047+
switch (TargetPlatform) {
3048+
default:
3049+
RequiresSubdirectorySearch = true;
3050+
break;
3051+
case MacOS:
3052+
RequiresSubdirectorySearch = SDKVersion < VersionTuple(15, 0);
3053+
break;
3054+
case IPhoneOS:
3055+
case TvOS:
3056+
RequiresSubdirectorySearch = SDKVersion < VersionTuple(18, 0);
3057+
break;
3058+
case WatchOS:
3059+
RequiresSubdirectorySearch = SDKVersion < VersionTuple(11, 0);
3060+
break;
3061+
case XROS:
3062+
RequiresSubdirectorySearch = SDKVersion < VersionTuple(2, 0);
3063+
break;
3064+
}
3065+
if (!RequiresSubdirectorySearch)
3066+
CC1Args.push_back("-fno-modulemap-allow-subdirectory-search");
3067+
}
30393068
}
30403069

30413070
void Darwin::addClangCC1ASTargetOptions(

‎clang/lib/Lex/HeaderSearch.cpp‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -378,20 +378,22 @@ Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName,
378378
break;
379379
}
380380

381-
// If we've already performed the exhaustive search for module maps in this
382-
// search directory, don't do it again.
383-
if (Dir.haveSearchedAllModuleMaps())
384-
continue;
381+
if (HSOpts->AllowModuleMapSubdirectorySearch) {
382+
// If we've already performed the exhaustive search for module maps in
383+
// this search directory, don't do it again.
384+
if (Dir.haveSearchedAllModuleMaps())
385+
continue;
385386

386-
// Load all module maps in the immediate subdirectories of this search
387-
// directory if ModuleName was from @import.
388-
if (AllowExtraModuleMapSearch)
389-
loadSubdirectoryModuleMaps(Dir);
387+
// Load all module maps in the immediate subdirectories of this search
388+
// directory if ModuleName was from @import.
389+
if (AllowExtraModuleMapSearch)
390+
loadSubdirectoryModuleMaps(Dir);
390391

391-
// Look again for the module.
392-
Module = ModMap.findModule(ModuleName);
393-
if (Module)
394-
break;
392+
// Look again for the module.
393+
Module = ModMap.findModule(ModuleName);
394+
if (Module)
395+
break;
396+
}
395397
}
396398

397399
return Module;

‎clang/test/Driver/modulemap-allow-subdirectory-search.c‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
4+
// Check that with a sufficiently new SDK not searching for module maps in subdirectories.
5+
6+
// New SDK.
7+
// RUN: %clang -target x86_64-apple-macos10.13 -isysroot %t/MacOSX15.0.sdk -fmodules %t/test.c -### 2>&1 \
8+
// RUN: | FileCheck --check-prefix=NO-SUBDIRECTORIES %t/test.c
9+
// Old SDK.
10+
// RUN: %clang -target x86_64-apple-macos10.13 -isysroot %t/MacOSX14.0.sdk -fmodules %t/test.c -### 2>&1 \
11+
// RUN: | FileCheck --check-prefix=SEARCH-SUBDIRECTORIES %t/test.c
12+
// Non-Darwin platform.
13+
// RUN: %clang -target i386-unknown-linux -isysroot %t/MacOSX15.0.sdk -fmodules %t/test.c -### 2>&1 \
14+
// RUN: | FileCheck --check-prefix=SEARCH-SUBDIRECTORIES %t/test.c
15+
// New SDK overriding the default.
16+
// RUN: %clang -target x86_64-apple-macos10.13 -isysroot %t/MacOSX15.0.sdk -fmodules %t/test.c -fmodulemap-allow-subdirectory-search -### 2>&1 \
17+
// RUN: | FileCheck --check-prefix=SEARCH-SUBDIRECTORIES %t/test.c
18+
19+
//--- test.c
20+
// NO-SUBDIRECTORIES: "-fno-modulemap-allow-subdirectory-search"
21+
// SEARCH-SUBDIRECTORIES-NOT: "-fno-modulemap-allow-subdirectory-search"
22+
23+
//--- MacOSX15.0.sdk/SDKSettings.json
24+
{"Version":"15.0", "MaximumDeploymentTarget": "15.0.99"}
25+
26+
//--- MacOSX14.0.sdk/SDKSettings.json
27+
{"Version":"14.0", "MaximumDeploymentTarget": "14.0.99"}

‎clang/test/Modules/modulemap-allow-subdirectory-search.m‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
4+
// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -I %t/include %t/test.m
5+
// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -I %t/include %t/test.m -fmodulemap-allow-subdirectory-search
6+
// RUN: not %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -I %t/include %t/test.m -fno-modulemap-allow-subdirectory-search
7+
8+
//--- include/UnrelatedName/Header.h
9+
// empty
10+
11+
//--- include/UnrelatedName/module.modulemap
12+
module UsefulCode {
13+
header "Header.h"
14+
export *
15+
}
16+
17+
//--- test.m
18+
@import UsefulCode;

0 commit comments

Comments
 (0)