From 970b2819122c1c79cc17033b679e7bf9dd479703 Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes Date: Tue, 20 Mar 2018 22:36:39 +0000 Subject: [PATCH] [Modules] Honor -fmodule-name when handling private framework modules When skipping building the module for a private framework module, LangOpts.CurrentModule isn't enough for implict modules builds; for instance, in case a private module is built while building a public one, LangOpts.CurrentModule doesn't reflect the -fmodule-name being passed down, but instead the module name which triggered the build. Store the actual -fmodule-name in LangOpts.ModuleName and actually check a name was provided during compiler invocation in order to skip building the private module. rdar://problem/38434694 llvm-svn: 328053 --- clang/include/clang/Basic/LangOptions.h | 6 ++++- clang/lib/Frontend/CompilerInstance.cpp | 4 ++++ clang/lib/Frontend/CompilerInvocation.cpp | 3 ++- clang/lib/Lex/PPDirectives.cpp | 26 +++++++++++++--------- .../Bad.framework/Headers/Bad.h | 1 + .../Bad.framework/Modules/module.modulemap | 5 +++++ .../Bad.framework/Modules/module.private.modulemap | 5 +++++ .../Bad.framework/PrivateHeaders/BadPrivate.h | 1 + .../Bad.framework/PrivateHeaders/Shared.h | 3 +++ clang/test/Modules/bad-private-include.m | 6 +++++ 10 files changed, 47 insertions(+), 13 deletions(-) create mode 100644 clang/test/Modules/Inputs/bad-private-include/Bad.framework/Headers/Bad.h create mode 100644 clang/test/Modules/Inputs/bad-private-include/Bad.framework/Modules/module.modulemap create mode 100644 clang/test/Modules/Inputs/bad-private-include/Bad.framework/Modules/module.private.modulemap create mode 100644 clang/test/Modules/Inputs/bad-private-include/Bad.framework/PrivateHeaders/BadPrivate.h create mode 100644 clang/test/Modules/Inputs/bad-private-include/Bad.framework/PrivateHeaders/Shared.h create mode 100644 clang/test/Modules/bad-private-include.m diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index b16392d..42ec9f3 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -141,10 +141,14 @@ public: /// If none is specified, abort (GCC-compatible behaviour). std::string OverflowHandler; + /// The module currently being compiled as speficied by -fmodule-name. + std::string ModuleName; + /// \brief The name of the current module, of which the main source file /// is a part. If CompilingModule is set, we are compiling the interface /// of this module, otherwise we are compiling an implementation file of - /// it. + /// it. This starts as ModuleName in case -fmodule-name is provided and + /// changes during compilation to reflect the current module. std::string CurrentModule; /// \brief The names of any features to enable in module 'requires' decls diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 306d7c0..bb8b857 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1090,6 +1090,10 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, }), PPOpts.Macros.end()); + // If the original compiler invocation had -fmodule-name, pass it through. + Invocation->getLangOpts()->ModuleName = + ImportingInstance.getInvocation().getLangOpts()->ModuleName; + // Note the name of the module we're building. Invocation->getLangOpts()->CurrentModule = ModuleName; diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 330212a..c9d251f 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -2372,7 +2372,8 @@ static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK, Opts.DebuggerCastResultToId = Args.hasArg(OPT_fdebugger_cast_result_to_id); Opts.DebuggerObjCLiteral = Args.hasArg(OPT_fdebugger_objc_literal); Opts.ApplePragmaPack = Args.hasArg(OPT_fapple_pragma_pack); - Opts.CurrentModule = Args.getLastArgValue(OPT_fmodule_name_EQ); + Opts.ModuleName = Args.getLastArgValue(OPT_fmodule_name_EQ); + Opts.CurrentModule = Opts.ModuleName; Opts.AppExt = Args.hasArg(OPT_fapplication_extension); Opts.ModuleFeatures = Args.getAllArgValues(OPT_fmodule_feature); std::sort(Opts.ModuleFeatures.begin(), Opts.ModuleFeatures.end()); diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index a901a20..0bd1b8b 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -115,18 +115,19 @@ static bool isReservedId(StringRef Text, const LangOptions &Lang) { return false; } -// The -fmodule-name option (represented here by \p CurrentModule) tells the -// compiler to textually include headers in the specified module, meaning clang -// won't build the specified module. This is useful in a number of situations, -// for instance, when building a library that vends a module map, one might want -// to avoid hitting intermediate build products containig the the module map or -// avoid finding the system installed modulemap for that library. -static bool isForModuleBuilding(Module *M, StringRef CurrentModule) { +// The -fmodule-name option tells the compiler to textually include headers in +// the specified module, meaning clang won't build the specified module. This is +// useful in a number of situations, for instance, when building a library that +// vends a module map, one might want to avoid hitting intermediate build +// products containig the the module map or avoid finding the system installed +// modulemap for that library. +static bool isForModuleBuilding(Module *M, StringRef CurrentModule, + StringRef ModuleName) { StringRef TopLevelName = M->getTopLevelModuleName(); // When building framework Foo, we wanna make sure that Foo *and* Foo_Private // are textually included and no modules are built for both. - if (M->getTopLevelModule()->IsFramework && + if (M->getTopLevelModule()->IsFramework && CurrentModule == ModuleName && !CurrentModule.endswith("_Private") && TopLevelName.endswith("_Private")) TopLevelName = TopLevelName.drop_back(8); @@ -1875,7 +1876,8 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, // are processing this module textually (because we're building the module). if (ShouldEnter && File && SuggestedModule && getLangOpts().Modules && !isForModuleBuilding(SuggestedModule.getModule(), - getLangOpts().CurrentModule)) { + getLangOpts().CurrentModule, + getLangOpts().ModuleName)) { // If this include corresponds to a module but that module is // unavailable, diagnose the situation and bail out. // FIXME: Remove this; loadModule does the same check (but produces @@ -2022,7 +2024,8 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, // ShouldEnter is false because we are skipping the header. In that // case, We are not importing the specified module. if (SkipHeader && getLangOpts().CompilingPCH && - isForModuleBuilding(M, getLangOpts().CurrentModule)) + isForModuleBuilding(M, getLangOpts().CurrentModule, + getLangOpts().ModuleName)) return; makeModuleVisible(M, HashLoc); @@ -2063,7 +2066,8 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, // include headers in the specified module. We are not building the // specified module. if (getLangOpts().CompilingPCH && - isForModuleBuilding(M, getLangOpts().CurrentModule)) + isForModuleBuilding(M, getLangOpts().CurrentModule, + getLangOpts().ModuleName)) return; assert(!CurLexerSubmodule && "should not have marked this as a module yet"); diff --git a/clang/test/Modules/Inputs/bad-private-include/Bad.framework/Headers/Bad.h b/clang/test/Modules/Inputs/bad-private-include/Bad.framework/Headers/Bad.h new file mode 100644 index 0000000..eea0549 --- /dev/null +++ b/clang/test/Modules/Inputs/bad-private-include/Bad.framework/Headers/Bad.h @@ -0,0 +1 @@ +#import \ No newline at end of file diff --git a/clang/test/Modules/Inputs/bad-private-include/Bad.framework/Modules/module.modulemap b/clang/test/Modules/Inputs/bad-private-include/Bad.framework/Modules/module.modulemap new file mode 100644 index 0000000..11612dc --- /dev/null +++ b/clang/test/Modules/Inputs/bad-private-include/Bad.framework/Modules/module.modulemap @@ -0,0 +1,5 @@ +framework module Bad { + umbrella header "Bad.h" + export * + module * { export * } +} diff --git a/clang/test/Modules/Inputs/bad-private-include/Bad.framework/Modules/module.private.modulemap b/clang/test/Modules/Inputs/bad-private-include/Bad.framework/Modules/module.private.modulemap new file mode 100644 index 0000000..043139d --- /dev/null +++ b/clang/test/Modules/Inputs/bad-private-include/Bad.framework/Modules/module.private.modulemap @@ -0,0 +1,5 @@ +framework module Bad_Private { + umbrella header "BadPrivate.h" + export * + module * { export * } +} diff --git a/clang/test/Modules/Inputs/bad-private-include/Bad.framework/PrivateHeaders/BadPrivate.h b/clang/test/Modules/Inputs/bad-private-include/Bad.framework/PrivateHeaders/BadPrivate.h new file mode 100644 index 0000000..eea0549 --- /dev/null +++ b/clang/test/Modules/Inputs/bad-private-include/Bad.framework/PrivateHeaders/BadPrivate.h @@ -0,0 +1 @@ +#import \ No newline at end of file diff --git a/clang/test/Modules/Inputs/bad-private-include/Bad.framework/PrivateHeaders/Shared.h b/clang/test/Modules/Inputs/bad-private-include/Bad.framework/PrivateHeaders/Shared.h new file mode 100644 index 0000000..4f1defd --- /dev/null +++ b/clang/test/Modules/Inputs/bad-private-include/Bad.framework/PrivateHeaders/Shared.h @@ -0,0 +1,3 @@ +struct SomeDecl { + int x; +}; diff --git a/clang/test/Modules/bad-private-include.m b/clang/test/Modules/bad-private-include.m new file mode 100644 index 0000000..e12b280 --- /dev/null +++ b/clang/test/Modules/bad-private-include.m @@ -0,0 +1,6 @@ +// RUN: rm -rf %t.cache +// RUN: %clang_cc1 -fmodules-cache-path=%t.cache -fmodules -fimplicit-module-maps -F %S/Inputs/bad-private-include %s -verify + +// expected-no-diagnostics + +@import Bad; -- 2.7.4