Allow use of private headers in different sub-modules.

[rsmith]

lib/Lex/ModuleMap.cpp

235–237

We still have some duplication between this check...

264–267

... and this one. I don't think we need both.

[klimek]

Remove unnecessary if and fix segfault.

[klimek]

Addressed comments.

[Eugene.Zelenko]

Committed in rL252170.

lib/Lex/ModuleMap.cpp

if (IsPrivateRole) {
     for (auto *Hs : HeaderList)
       IsPrivate |=
           std::find_if(Hs->begin(), Hs->end(), [&](const Module::Header &H) {
             return H.Entry == IncFileEnt;
           }) != Hs->end();
     assert((!IsPrivateRole || IsPrivate) && "inconsistent headers and roles");
   }
 #endif
-  return IsPrivateRole &&
-         // FIXME: Should we map RequestingModule to its top-level module here
-         //        too? This check is redundant with the isSubModuleOf check in
-         //        diagnoseHeaderInclusion.
-         RequestedModule->getTopLevelModule() != RequestingModule;
+  return IsPrivateRole && (!RequestingModule ||
+                           RequestedModule->getTopLevelModule() !=
+                               RequestingModule->getTopLevelModule());
 }
 
 static Module *getTopLevelOrNull(Module *M) {
   return M ? M->getTopLevelModule() : nullptr;
 }
 
 void ModuleMap::diagnoseHeaderInclusion(Module *RequestingModule,
                                         SourceLocation FilenameLoc,
void ModuleMap::diagnoseHeaderInclusion(Module *RequestingModule,
 
   bool Excluded = false;
   Module *Private = nullptr;
   Module *NotUsed = nullptr;
 
   HeadersMap::iterator Known = findKnownHeader(File);
   if (Known != Headers.end()) {
     for (const KnownHeader &Header : Known->second) {
-      // If 'File' is part of 'RequestingModule' we can definitely include it.
-      if (Header.getModule() &&
-          Header.getModule()->isSubModuleOf(RequestingModule))
-        return;
-
       // Remember private headers for later printing of a diagnostic.
       if (violatesPrivateInclude(RequestingModule, File, Header.getRole(),
                                  Header.getModule())) {
         Private = Header.getModule();
         continue;
       }
 
       // If uses need to be specified explicitly, we are only allowed to return

test/Modules/Inputs/private3/private.h

+#ifndef PRIVATE_H
+#define PRIVATE_H
+
+void priv();
+
+#endif
+

test/Modules/Inputs/private3/public.h

+#ifndef PUBLIC_H
+#define PUBLIC_H
+
+#include "private.h"
+
+void pub() {
+  priv();
+}
+
+#endif
+

test/Modules/private.modulemap

+// RUN: rm -rf %t
+// RUN: cd %S
+// RUN: %clang_cc1 -fmodules -x c++ -std=c++11 -fmodules-cache-path=%t \
+// RUN:   -I%S/Inputs/private3 -emit-module -fmodule-name=A -o %t/m.pcm %s
+// RUN: %clang_cc1 -fmodules -x c++ -std=c++11 -fmodules-cache-path=%t \
+// RUN:   -I%S/Inputs/private3 -emit-module -fmodule-name=B -o %t/m.pcm %s
+// RUN: %clang_cc1 -fmodules -x c++ -std=c++11 -fmodules-cache-path=%t \
+// RUN:   -I%S/Inputs/private3 -emit-module -fmodule-name=C -o %t/m.pcm %s
+// RUN: %clang_cc1 -fmodules -x c++ -std=c++11 -fmodules-cache-path=%t \
+// RUN:   -I%S/Inputs/private3 -emit-module -fmodule-name=D -o %t/m.pcm %s
+
+module A {
+  header "Inputs/private3/public.h"
+  private header "Inputs/private3/private.h"
+}
+module B {
+  header "Inputs/private3/public.h"
+  module "private.h" {
+    private header "Inputs/private3/private.h"
+  }
+}
+module C {
+  module "public.h" {
+    header "Inputs/private3/public.h"
+  }
+  private header "Inputs/private3/private.h"
+}
+module D {
+  module "public.h" {
+    header "Inputs/private3/public.h"
+  }
+  module "private.h" {
+    private header "Inputs/private3/private.h"
+  }
+}