Skip to content

Commit

Permalink
[clang][modules][deps] Parse "FW_Private" module map even after loadi…
Browse files Browse the repository at this point in the history
…ng "FW" PCM

When Clang loads a PCM that depends on another PCM describing framework module "FW", `ModuleMap` registers "FW" as known, without seeing the module map that defines it (or the adjacent "FW_Private" module map). Later, when looking at a header from "FW_Private", `ModuleMap` returns early due to having knowledge about "FW" and never associates that header with "FW_Private", leading to it being treated as textual. This behavior is caused by D150292, where the scanner stops calling `HeaderSearch::lookupModule()` eagerly for every loaded PCM.

This patch skips an early check when trying to figure out the framework module for a header, which ensures the "FW" and (most importantly) "FW_Private" module maps can be parsed even after loading "FW" from a PCM. Note that the `HeaderSearch::loadModuleMapFile()` function we not call unconditionally has caching behavior of its own, meaning it will avoid parsing module map file repeatedly.

Depends on D150320.

Reviewed By: benlangmuir

Differential Revision: https://reviews.llvm.org/D150478
  • Loading branch information
jansvoboda11 committed Jul 17, 2023
1 parent 227f719 commit be01456
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 4 deletions.
4 changes: 4 additions & 0 deletions clang/include/clang/Driver/Options.td
Expand Up @@ -2588,6 +2588,10 @@ defm modules_search_all : BoolFOption<"modules-search-all",
defm implicit_modules : BoolFOption<"implicit-modules",
LangOpts<"ImplicitModules">, DefaultTrue,
NegFlag<SetFalse, [CC1Option]>, PosFlag<SetTrue>, BothFlags<[NoXarchOption,CoreOption]>>;
def fno_modules_check_relocated : Joined<["-"], "fno-modules-check-relocated">,
Group<f_Group>, Flags<[CC1Option]>,
HelpText<"Skip checks for relocated modules when loading PCM files">,
MarshallingInfoNegativeFlag<PreprocessorOpts<"ModulesCheckRelocated">>;
def fretain_comments_from_system_headers : Flag<["-"], "fretain-comments-from-system-headers">, Group<f_Group>, Flags<[CC1Option]>,
MarshallingInfoFlag<LangOpts<"RetainCommentsFromSystemHeaders">>;
def fmodule_header : Flag <["-"], "fmodule-header">, Group<f_Group>,
Expand Down
5 changes: 1 addition & 4 deletions clang/lib/Lex/HeaderSearch.cpp
Expand Up @@ -1783,9 +1783,6 @@ HeaderSearch::lookupModuleMapFile(DirectoryEntryRef Dir, bool IsFramework) {

Module *HeaderSearch::loadFrameworkModule(StringRef Name, DirectoryEntryRef Dir,
bool IsSystem) {
if (Module *Module = ModMap.findModule(Name))
return Module;

// Try to load a module map file.
switch (loadModuleMapFile(Dir, IsSystem, /*IsFramework*/true)) {
case LMM_InvalidModuleMap:
Expand All @@ -1794,10 +1791,10 @@ Module *HeaderSearch::loadFrameworkModule(StringRef Name, DirectoryEntryRef Dir,
ModMap.inferFrameworkModule(Dir, IsSystem, /*Parent=*/nullptr);
break;

case LMM_AlreadyLoaded:
case LMM_NoDirectory:
return nullptr;

case LMM_AlreadyLoaded:
case LMM_NewlyLoaded:
break;
}
Expand Down
23 changes: 23 additions & 0 deletions clang/test/Modules/no-check-relocated-fw-private.c
@@ -0,0 +1,23 @@
// RUN: rm -rf %t
// RUN: split-file %s %t

//--- frameworks1/FW1.framework/Modules/module.modulemap
framework module FW1 { header "FW1.h" }
//--- frameworks1/FW1.framework/Headers/FW1.h
#import <FW2/FW2.h>

//--- frameworks2/FW2.framework/Modules/module.modulemap
framework module FW2 { header "FW2.h" }
//--- frameworks2/FW2.framework/Modules/module.private.modulemap
framework module FW2_Private { header "FW2_Private.h" }
//--- frameworks2/FW2.framework/Headers/FW2.h
//--- frameworks2/FW2.framework/PrivateHeaders/FW2_Private.h

//--- tu.c
#import <FW1/FW1.h> // expected-remark{{importing module 'FW1'}} \
// expected-remark{{importing module 'FW2'}}
#import <FW2/FW2_Private.h> // expected-remark{{importing module 'FW2_Private'}}

// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache -fimplicit-module-maps \
// RUN: -F %t/frameworks1 -F %t/frameworks2 -fsyntax-only %t/tu.c \
// RUN: -fno-modules-check-relocated -Rmodule-import -verify

0 comments on commit be01456

Please sign in to comment.