Skip to content

Commit

Permalink
[Modules] Don't search for modulemaps in the immediate sub-directorie…
Browse files Browse the repository at this point in the history
…s of search paths for recent Apple SDKs. (llvm#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 <jan@svoboda.ai>
  • Loading branch information
vsapsai and jansvoboda11 committed Jul 23, 2024
1 parent d0c8e26 commit cbd5ba2
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 13 deletions.
4 changes: 4 additions & 0 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -3232,6 +3232,10 @@ def fimplicit_module_maps : Flag <["-"], "fimplicit-module-maps">, Group<f_Group
Visibility<[ClangOption, CC1Option, CLOption]>,
HelpText<"Implicitly search the file system for module map files.">,
MarshallingInfoFlag<HeaderSearchOpts<"ImplicitModuleMaps">>;
defm modulemap_allow_subdirectory_search : BoolFOption <"modulemap-allow-subdirectory-search",
HeaderSearchOpts<"AllowModuleMapSubdirectorySearch">, DefaultTrue,
PosFlag<SetTrue, [], [], "Allow to search for module maps in subdirectories of search paths">,
NegFlag<SetFalse>, BothFlags<[NoXarchOption], [ClangOption, CC1Option]>>;
defm modules : BoolFOption<"modules",
LangOpts<"Modules">, Default<fcxx_modules.KeyPath>,
PosFlag<SetTrue, [], [ClangOption, CC1Option],
Expand Down
9 changes: 8 additions & 1 deletion clang/include/clang/Lex/HeaderSearchOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,12 @@ class HeaderSearchOptions {
LLVM_PREFERRED_TYPE(bool)
unsigned ModulesIncludeVFSUsage : 1;

/// Whether we should look for a module in module maps only in provided
/// header search paths or if we are allowed to look for module maps in
/// subdirectories of provided paths too.
LLVM_PREFERRED_TYPE(bool)
unsigned AllowModuleMapSubdirectorySearch : 1;

HeaderSearchOptions(StringRef _Sysroot = "/")
: Sysroot(_Sysroot), ModuleFormat("raw"), DisableModuleHash(false),
ImplicitModuleMaps(false), ModuleMapFileHomeIsCwd(false),
Expand All @@ -285,7 +291,8 @@ class HeaderSearchOptions {
ModulesSkipHeaderSearchPaths(false),
ModulesSkipPragmaDiagnosticMappings(false),
ModulesPruneNonAffectingModuleMaps(true), ModulesHashContent(false),
ModulesStrictContextHash(false), ModulesIncludeVFSUsage(false) {}
ModulesStrictContextHash(false), ModulesIncludeVFSUsage(false),
AllowModuleMapSubdirectorySearch(true) {}

/// AddPath - Add the \p Path path to the specified \p Group list.
void AddPath(StringRef Path, frontend::IncludeDirGroup Group,
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3960,6 +3960,9 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
options::OPT_fno_modules_strict_decluse, false))
CmdArgs.push_back("-fmodules-strict-decluse");

Args.addOptOutFlag(CmdArgs, options::OPT_fmodulemap_allow_subdirectory_search,
options::OPT_fno_modulemap_allow_subdirectory_search);

// -fno-implicit-modules turns off implicitly compiling modules on demand.
bool ImplicitModules = false;
if (!Args.hasFlag(options::OPT_fimplicit_modules,
Expand Down
29 changes: 29 additions & 0 deletions clang/lib/Driver/ToolChains/Darwin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3036,6 +3036,35 @@ void Darwin::addClangTargetOptions(
if (!DriverArgs.hasArgNoClaim(options::OPT_fdefine_target_os_macros,
options::OPT_fno_define_target_os_macros))
CC1Args.push_back("-fdefine-target-os-macros");

// Disable subdirectory modulemap search on sufficiently recent SDKs.
if (SDKInfo &&
!DriverArgs.hasFlag(options::OPT_fmodulemap_allow_subdirectory_search,
options::OPT_fno_modulemap_allow_subdirectory_search,
false)) {
bool RequiresSubdirectorySearch;
VersionTuple SDKVersion = SDKInfo->getVersion();
switch (TargetPlatform) {
default:
RequiresSubdirectorySearch = true;
break;
case MacOS:
RequiresSubdirectorySearch = SDKVersion < VersionTuple(15, 0);
break;
case IPhoneOS:
case TvOS:
RequiresSubdirectorySearch = SDKVersion < VersionTuple(18, 0);
break;
case WatchOS:
RequiresSubdirectorySearch = SDKVersion < VersionTuple(11, 0);
break;
case XROS:
RequiresSubdirectorySearch = SDKVersion < VersionTuple(2, 0);
break;
}
if (!RequiresSubdirectorySearch)
CC1Args.push_back("-fno-modulemap-allow-subdirectory-search");
}
}

void Darwin::addClangCC1ASTargetOptions(
Expand Down
26 changes: 14 additions & 12 deletions clang/lib/Lex/HeaderSearch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -378,20 +378,22 @@ Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName,
break;
}

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

// Load all module maps in the immediate subdirectories of this search
// directory if ModuleName was from @import.
if (AllowExtraModuleMapSearch)
loadSubdirectoryModuleMaps(Dir);
// Load all module maps in the immediate subdirectories of this search
// directory if ModuleName was from @import.
if (AllowExtraModuleMapSearch)
loadSubdirectoryModuleMaps(Dir);

// Look again for the module.
Module = ModMap.findModule(ModuleName);
if (Module)
break;
// Look again for the module.
Module = ModMap.findModule(ModuleName);
if (Module)
break;
}
}

return Module;
Expand Down
27 changes: 27 additions & 0 deletions clang/test/Driver/modulemap-allow-subdirectory-search.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// RUN: rm -rf %t
// RUN: split-file %s %t

// Check that with a sufficiently new SDK not searching for module maps in subdirectories.

// New SDK.
// RUN: %clang -target x86_64-apple-macos10.13 -isysroot %t/MacOSX15.0.sdk -fmodules %t/test.c -### 2>&1 \
// RUN: | FileCheck --check-prefix=NO-SUBDIRECTORIES %t/test.c
// Old SDK.
// RUN: %clang -target x86_64-apple-macos10.13 -isysroot %t/MacOSX14.0.sdk -fmodules %t/test.c -### 2>&1 \
// RUN: | FileCheck --check-prefix=SEARCH-SUBDIRECTORIES %t/test.c
// Non-Darwin platform.
// RUN: %clang -target i386-unknown-linux -isysroot %t/MacOSX15.0.sdk -fmodules %t/test.c -### 2>&1 \
// RUN: | FileCheck --check-prefix=SEARCH-SUBDIRECTORIES %t/test.c
// New SDK overriding the default.
// RUN: %clang -target x86_64-apple-macos10.13 -isysroot %t/MacOSX15.0.sdk -fmodules %t/test.c -fmodulemap-allow-subdirectory-search -### 2>&1 \
// RUN: | FileCheck --check-prefix=SEARCH-SUBDIRECTORIES %t/test.c

//--- test.c
// NO-SUBDIRECTORIES: "-fno-modulemap-allow-subdirectory-search"
// SEARCH-SUBDIRECTORIES-NOT: "-fno-modulemap-allow-subdirectory-search"

//--- MacOSX15.0.sdk/SDKSettings.json
{"Version":"15.0", "MaximumDeploymentTarget": "15.0.99"}

//--- MacOSX14.0.sdk/SDKSettings.json
{"Version":"14.0", "MaximumDeploymentTarget": "14.0.99"}
18 changes: 18 additions & 0 deletions clang/test/Modules/modulemap-allow-subdirectory-search.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// RUN: rm -rf %t
// RUN: split-file %s %t

// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -I %t/include %t/test.m
// 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
// 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

//--- include/UnrelatedName/Header.h
// empty

//--- include/UnrelatedName/module.modulemap
module UsefulCode {
header "Header.h"
export *
}

//--- test.m
@import UsefulCode;

0 comments on commit cbd5ba2

Please sign in to comment.