[clang][modules] Consider M affecting after mapping M.Private to M_Private

[jansvoboda11]

Summary

Wwhen Clang encounters @import M.Private during implicit build, it precompiles module M and looks through its submodules. If the Private submodule is not found, Clang assumes @import M_Private. In the dependency scanner, we don't capture the dependency on M, since
it's not imported. It's an affecting module, though: compilation of the import statement will fail when implicit modules are disabled and M is not precompiled and explicitly provided. This patch fixes that.

[jansvoboda11]

Arcanist created two revisions, removing this one.

clang/include/clang/Lex/Preprocessor.h

public:
   macros(bool IncludeExternalMacros = true) const {
     macro_iterator begin = macro_begin(IncludeExternalMacros);
     macro_iterator end = macro_end(IncludeExternalMacros);
     return llvm::make_range(begin, end);
   }
 
   /// \}
 
+  /// Mark the given module as affecting the current module or translation unit.
+  void markModuleAsAffecting(Module *M) {
+    if (!BuildingSubmoduleStack.empty()) {
+      if (M != BuildingSubmoduleStack.back().M)
+        BuildingSubmoduleStack.back().M->AffectingModules.insert(M);
+    } else {
+      AffectingModules.insert(M);
+    }
+  }
+
   /// Get the set of top-level modules that affected preprocessing, but were not
   /// imported.
   const llvm::SmallSetVector<Module *, 2> &getAffectingModules() const {
     return AffectingModules;
   }
 
   /// Mark the file as included.
   /// Returns true if this is the first time the file was included.

clang/lib/Frontend/CompilerInstance.cpp

CompilerInstance::loadModule(SourceLocation ImportLoc,
   for (unsigned I = 1, N = Path.size(); I != N; ++I) {
     StringRef Name = Path[I].first->getName();
     clang::Module *Sub = Module->findSubmodule(Name);
 
     // If the user is requesting Foo.Private and it doesn't exist, try to
     // match Foo_Private and emit a warning asking for the user to write
     // @import Foo_Private instead. FIXME: remove this when existing clients
     // migrate off of Foo.Private syntax.
-    if (!Sub && PP->getLangOpts().ImplicitModules && Name == "Private" &&
-        Module == Module->getTopLevelModule()) {
+    if (!Sub && Name == "Private" && Module == Module->getTopLevelModule()) {
       SmallString<128> PrivateModule(Module->Name);
       PrivateModule.append("_Private");
 
       SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> PrivPath;
       auto &II = PP->getIdentifierTable().get(
           PrivateModule, PP->getIdentifierInfo(Module->Name)->getTokenID());
       PrivPath.push_back(std::make_pair(&II, Path[0].second));
 
       if (PP->getHeaderSearchInfo().lookupModule(PrivateModule, ImportLoc, true,
                                                  !IsInclusionDirective))
         Sub = loadModule(ImportLoc, PrivPath, Visibility, IsInclusionDirective);
       if (Sub) {
         MapPrivateSubModToTopLevel = true;
+        getPreprocessor().markModuleAsAffecting(Module);
         if (!getDiagnostics().isIgnored(
                 diag::warn_no_priv_submodule_use_toplevel, ImportLoc)) {
           getDiagnostics().Report(Path[I].second,
                                   diag::warn_no_priv_submodule_use_toplevel)
               << Path[I].first << Module->getFullModuleName() << PrivateModule
               << SourceRange(Path[0].second, Path[I].second)
               << FixItHint::CreateReplacement(SourceRange(Path[0].second),
                                               PrivateModule);

clang/lib/Lex/PPDirectives.cpp

ModuleLoadResult Imported = TheModuleLoader.loadModule(
         IncludeTok.getLocation(), Path, Module::Hidden,
         /*IsInclusionDirective=*/true);
     assert((Imported == nullptr || Imported == SuggestedModule.getModule()) &&
            "the imported module is different than the suggested one");
 
     if (Imported) {
       Action = Import;
     } else if (Imported.isMissingExpected()) {
-      Module *M = static_cast<Module *>(Imported)->getTopLevelModule();
-      if (!BuildingSubmoduleStack.empty()) {
-        if (Imported != BuildingSubmoduleStack.back().M)
-          BuildingSubmoduleStack.back().M->AffectingModules.insert(M);
-      } else {
-        AffectingModules.insert(M);
-      }
+      markModuleAsAffecting(
+          static_cast<Module *>(Imported)->getTopLevelModule());
       // We failed to find a submodule that we assumed would exist (because it
       // was in the directory of an umbrella header, for instance), but no
       // actual module containing it exists (because the umbrella header is
       // incomplete).  Treat this as a textual inclusion.
       SuggestedModule = ModuleMap::KnownHeader();
       SM = nullptr;
     } else if (Imported.isConfigMismatch()) {
       // On a configuration mismatch, enter the header textually. We still know

clang/test/Modules/implicit-map-dot-private.m

 // RUN: rm -rf %t
-// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-canonical -emit-pch -o %t-A.pch %s -Wprivate-module
+
+// Implicit modules.
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:   -F %S/Inputs/implicit-private-canonical -fsyntax-only %s -Wprivate-module
+
+// Explicit modules.
+// RUN: %clang_cc1 -x objective-c -fmodules -emit-module -fmodule-name=A -o %t/A.pcm \
+// RUN:   %S/Inputs/implicit-private-canonical/A.framework/Modules/module.modulemap -Wprivate-module
+// RUN: %clang_cc1 -x objective-c -fmodules -emit-module -fmodule-name=A_Private -o %t/A_Private.pcm \
+// RUN:   %S/Inputs/implicit-private-canonical/A.framework/Modules/module.private.modulemap -Wprivate-module
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fno-implicit-modules \
+// RUN:   -fmodule-file=A=%t/A.pcm -fmodule-file=A_Private=%t/A_Private.pcm \
+// RUN:   -F %S/Inputs/implicit-private-canonical -fsyntax-only %s -Wprivate-module
 
 #ifndef HEADER
 #define HEADER
 
 @import A.Private; // expected-warning {{no submodule named 'Private' in module 'A'; using top level 'A_Private'}}
 // expected-note@Inputs/implicit-private-canonical/A.framework/Modules/module.private.modulemap:1{{module defined here}}
 
 const int *y = &APRIVATE;
 
 #endif