[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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@328053 91177308-0d34-0410-b5e6-96231b3b80d8

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

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;
 

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());

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");

test/Modules/Inputs/bad-private-include/Bad.framework/Headers/Bad.h

@@ -0,0 +1 @@
+#import <Bad/Shared.h>

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 * }
+}

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 * }
+}

test/Modules/Inputs/bad-private-include/Bad.framework/PrivateHeaders/BadPrivate.h

@@ -0,0 +1 @@
+#import <Bad/Shared.h>

test/Modules/Inputs/bad-private-include/Bad.framework/PrivateHeaders/Shared.h

@@ -0,0 +1,3 @@
+struct SomeDecl {
+  int x;
+};

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;