r231728 - [modules] This check is run before we resolve the header, not after, so just

[Richard Smith]

Author: rsmith
Date: Mon Mar  9 19:19:04 2015
New Revision: 231728

URL: http://llvm.org/viewvc/llvm-project?rev=231728&view=rev
Log:
[modules] This check is run before we resolve the header, not after, so just
check that private headers are in a list matching the role. (We can't perform
the opposite checks for non-private headers because we infer those.)

Modified:
    cfe/trunk/lib/Lex/ModuleMap.cpp
    cfe/trunk/test/Modules/public-private.modulemap

Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=231728&r1=231727&r2=231728&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Mon Mar  9 19:19:04 2015
@@ -219,28 +219,21 @@ static bool violatesPrivateInclude(Modul
                                    Module *RequestedModule) {
   bool IsPrivateRole = Role & ModuleMap::PrivateHeader;
 #ifndef NDEBUG
-  // Check for consistency between the module header role
-  // as obtained from the lookup and as obtained from the module.
-  // This check is not cheap, so enable it only for debugging.
-  auto IsInHeaderList = [&](std::initializer_list<SmallVectorImpl<
-                                Module::Header>*> HeaderList) -> bool {
-    for (auto *Hs : HeaderList) {
-      if (std::find_if(Hs->begin(), Hs->end(), [&](const Module::Header &H) {
+  if (IsPrivateRole) {
+    // Check for consistency between the module header role
+    // as obtained from the lookup and as obtained from the module.
+    // This check is not cheap, so enable it only for debugging.
+    bool IsPrivate = false;
+    SmallVectorImpl<Module::Header> *HeaderList[] = {
+        &RequestedModule->Headers[Module::HK_Private],
+        &RequestedModule->Headers[Module::HK_PrivateTextual]};
+    for (auto *Hs : HeaderList)
+      IsPrivate |=
+          std::find_if(Hs->begin(), Hs->end(), [&](const Module::Header &H) {
             return H.Entry == IncFileEnt;
-          }) != Hs->end())
-        return true;
-    }
-    return false;
-  };
-  // If a header is both public and private, then it's available as a public
-  // header and that's OK.
-  // FIXME: Should we reject this when parsing the module map?
-  bool IsPrivate =
-      IsInHeaderList({&RequestedModule->Headers[Module::HK_Private],
-                      &RequestedModule->Headers[Module::HK_PrivateTextual]}) &&
-      !IsInHeaderList({&RequestedModule->Headers[Module::HK_Normal],
-                       &RequestedModule->Headers[Module::HK_Textual]});
-  assert(IsPrivate == IsPrivateRole && "inconsistent headers and roles");
+          }) != Hs->end();
+    assert((!IsPrivateRole || IsPrivate) && "inconsistent headers and roles");
+  }
 #endif
   return IsPrivateRole &&
          RequestedModule->getTopLevelModule() != RequestingModule;

Modified: cfe/trunk/test/Modules/public-private.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/public-private.modulemap?rev=231728&r1=231727&r2=231728&view=diff
==============================================================================
--- cfe/trunk/test/Modules/public-private.modulemap (original)
+++ cfe/trunk/test/Modules/public-private.modulemap Mon Mar  9 19:19:04 2015
@@ -1,5 +1,10 @@
 // RUN: %clang_cc1 -fmodules -fmodule-map-file=%s -I%S -include "Inputs/empty.h" /dev/null
-module Blah {
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%s -I%S -include "Inputs/dummy.h" /dev/null
+module A {
   header "Inputs/empty.h"
   private header "Inputs/empty.h"
 }
+module B {
+  private header "Inputs/dummy.h"
+  header "Inputs/dummy.h"
+}