[Modules] Change private modules rules and warnings

We used to advertise private modules to be declared as submodules
(Foo.Private). This has proven to not scale well since private headers
might carry several dependencies, introducing unwanted content into the
main module and often causing dep cycles.

Change the canonical way to name it to Foo_Private, forcing private
modules as top level ones, and provide warnings under -Wprivate-module
to suggest fixes for other private naming. Update documentation to
reflect that.

rdar://problem/31173501

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

diff --git a/docs/Modules.rst b/docs/Modules.rst
index 757be619139094de8efa5bac53a1e4c211d05e9d..2fa38be6f4930107c9bfaac52cb7cd8703e7a625 100644 (file)
--- a/docs/Modules.rst
+++ b/docs/Modules.rst
@@ -859,10 +859,12 @@ express this with a single module map file in the library:
 
   module Foo {
     header "Foo.h"
-    
-    explicit module Private {
-      header "Foo_Private.h"
-    }
+    ...
+  }
+
+  module Foo_Private {
+    header "Foo_Private.h"
+    ...
   }
 
 
@@ -873,7 +875,7 @@ build machinery.
 
 Private module map files, which are named ``module.private.modulemap``
 (or, for backward compatibility, ``module_private.map``), allow one to
-augment the primary module map file with an additional submodule. For
+augment the primary module map file with an additional modules. For
 example, we would split the module map file above into two module map
 files:
 
@@ -883,9 +885,9 @@ files:
   module Foo {
     header "Foo.h"
   }
-  
+
   /* module.private.modulemap */
-  explicit module Foo.Private {
+  module Foo_Private {
     header "Foo_Private.h"
   }
 
@@ -899,13 +901,12 @@ boundaries.
 
 When writing a private module as part of a *framework*, it's recommended that:
 
-* Headers for this module are present in the ``PrivateHeaders``
-  framework subdirectory.
-* The private module is defined as a *submodule* of the public framework (if
-  there's one), similar to how ``Foo.Private`` is defined in the example above.
-* The ``explicit`` keyword should be used to guarantee that its content will
-  only be available when the submodule itself is explicitly named (through a
-  ``@import`` for example).
+* Headers for this module are present in the ``PrivateHeaders`` framework
+  subdirectory.
+* The private module is defined as a *top level module* with the name of the
+  public framework prefixed, like ``Foo_Private`` above. Clang has extra logic
+  to work with this naming, using ``FooPrivate`` or ``Foo.Private`` (submodule)
+  trigger warnings and might not work as expected.
 
 Modularizing a Platform
 =======================

diff --git a/include/clang/Basic/DiagnosticLexKinds.td b/include/clang/Basic/DiagnosticLexKinds.td
index c664281ffcd48bc676614c4cc610f3e19215afe3..c391470cb1c897dc90591a124d498ca92928126d 100644 (file)
--- a/include/clang/Basic/DiagnosticLexKinds.td
+++ b/include/clang/Basic/DiagnosticLexKinds.td
@@ -691,11 +691,15 @@ def err_mmap_expected_feature : Error<"expected a feature name">;
 def err_mmap_expected_attribute : Error<"expected an attribute name">;
 def warn_mmap_unknown_attribute : Warning<"unknown attribute '%0'">,
   InGroup<IgnoredAttributes>;
-def warn_mmap_mismatched_top_level_private : Warning<
-  "top-level module '%0' in private module map, expected a submodule of '%1'">,
+def warn_mmap_mismatched_private_submodule : Warning<
+  "private submodule '%0' in private module map, expected top-level module">,
   InGroup<PrivateModule>;
-def note_mmap_rename_top_level_private_as_submodule : Note<
-  "make '%0' a submodule of '%1' to ensure it can be found by name">;
+def warn_mmap_mismatched_private_module_name : Warning<
+  "expected canonical name for private module '%0'">,
+  InGroup<PrivateModule>;
+def note_mmap_rename_top_level_private_module : Note<
+  "rename '%0' to ensure it can be found by name">;
+
 def err_mmap_duplicate_header_attribute : Error<
   "header attribute '%0' specified multiple times">;
 def err_mmap_invalid_header_attribute_value : Error<

diff --git a/lib/Lex/HeaderSearch.cpp b/lib/Lex/HeaderSearch.cpp
index aa2588659ddfe30a519622c50601fa6558f9e9c7..6976294a2eaf3e7c89165e173d5833aa4527b124 100644 (file)
--- a/lib/Lex/HeaderSearch.cpp
+++ b/lib/Lex/HeaderSearch.cpp
@@ -209,11 +209,14 @@ Module *HeaderSearch::lookupModule(StringRef ModuleName, bool AllowSearch) {
 
   // The facility for "private modules" -- adjacent, optional module maps named
   // module.private.modulemap that are supposed to define private submodules --
-  // is sometimes misused by frameworks that name their associated private
-  // module FooPrivate, rather than as a submodule named Foo.Private as
-  // intended. Here we compensate for such cases by looking in directories named
-  // Foo.framework, when we previously looked and failed to find a
-  // FooPrivate.framework.
+  // may have different flavors of names: FooPrivate, Foo_Private and Foo.Private.
+  //
+  // Foo.Private is now depracated in favor of Foo_Private. Users of FooPrivate
+  // should also rename to Foo_Private. Representing private as submodules
+  // could force building unwanted dependencies into the parent module and cause
+  // dependency cycles.
+  if (!Module && SearchName.consume_back("_Private"))
+    Module = lookupModule(ModuleName, SearchName);
   if (!Module && SearchName.consume_back("Private"))
     Module = lookupModule(ModuleName, SearchName);
   return Module;

diff --git a/lib/Lex/ModuleMap.cpp b/lib/Lex/ModuleMap.cpp
index fbbae7a095203b275f8d473e5ec9fa415d09b67a..b3ac10c5c5ae85d61f290cf547b4cf2171afc6f2 100644 (file)
--- a/lib/Lex/ModuleMap.cpp
+++ b/lib/Lex/ModuleMap.cpp
@@ -1608,6 +1608,54 @@ namespace {
 
 } // namespace
 
+/// Private modules are canonicalized as Foo_Private. Clang provides extra
+/// module map search logic to find the appropriate private module when PCH
+/// is used with implicit module maps. Warn when private modules are written
+/// in other ways (FooPrivate and Foo.Private), providing notes and fixits.
+static void diagnosePrivateModules(const ModuleMap &Map,
+                                   DiagnosticsEngine &Diags,
+                                   const Module *ActiveModule) {
+
+  auto GenNoteAndFixIt = [&](StringRef BadName, StringRef Canonical,
+                             const Module *M) {
+    auto D = Diags.Report(ActiveModule->DefinitionLoc,
+                          diag::note_mmap_rename_top_level_private_module);
+    D << BadName << M->Name;
+    D << FixItHint::CreateReplacement(ActiveModule->DefinitionLoc, Canonical);
+  };
+
+  for (auto E = Map.module_begin(); E != Map.module_end(); ++E) {
+    auto const *M = E->getValue();
+    if (M->Directory != ActiveModule->Directory)
+      continue;
+
+    SmallString<128> FullName(ActiveModule->getFullModuleName());
+    if (!FullName.startswith(M->Name) && !FullName.endswith("Private"))
+      continue;
+    SmallString<128> Canonical(M->Name);
+    Canonical.append("_Private");
+
+    // Foo.Private -> Foo_Private
+    if (ActiveModule->Parent && ActiveModule->Name == "Private" && !M->Parent &&
+        M->Name == ActiveModule->Parent->Name) {
+      Diags.Report(ActiveModule->DefinitionLoc,
+                   diag::warn_mmap_mismatched_private_submodule)
+          << FullName;
+      GenNoteAndFixIt(FullName, Canonical, M);
+      continue;
+    }
+
+    // FooPrivate and whatnots -> Foo_Private
+    if (!ActiveModule->Parent && !M->Parent && M->Name != ActiveModule->Name &&
+        ActiveModule->Name != Canonical) {
+      Diags.Report(ActiveModule->DefinitionLoc,
+                   diag::warn_mmap_mismatched_private_module_name)
+          << ActiveModule->Name;
+      GenNoteAndFixIt(ActiveModule->Name, Canonical, M);
+    }
+  }
+}
+
 /// \brief Parse a module declaration.
 ///
 ///   module-declaration:
@@ -1791,41 +1839,21 @@ void ModuleMapParser::parseModuleDecl() {
     ActiveModule->NoUndeclaredIncludes = true;
   ActiveModule->Directory = Directory;
 
-  if (!ActiveModule->Parent) {
-    StringRef MapFileName(ModuleMapFile->getName());
-    if (MapFileName.endswith("module.private.modulemap") ||
-        MapFileName.endswith("module_private.map")) {
-      // Adding a top-level module from a private modulemap is likely a
-      // user error; we check to see if there's another top-level module
-      // defined in the non-private map in the same dir, and if so emit a
-      // warning.
-      for (auto E = Map.module_begin(); E != Map.module_end(); ++E) {
-        auto const *M = E->getValue();
-        if (!M->Parent &&
-            M->Directory == ActiveModule->Directory &&
-            M->Name != ActiveModule->Name) {
-          Diags.Report(ActiveModule->DefinitionLoc,
-                       diag::warn_mmap_mismatched_top_level_private)
-            << ActiveModule->Name << M->Name;
-          // The pattern we're defending against here is typically due to
-          // a module named FooPrivate which is supposed to be a submodule
-          // called Foo.Private. Emit a fixit in that case.
-          auto D =
-            Diags.Report(ActiveModule->DefinitionLoc,
-                         diag::note_mmap_rename_top_level_private_as_submodule);
-          D << ActiveModule->Name << M->Name;
-          StringRef Bad(ActiveModule->Name);
-          if (Bad.consume_back("Private")) {
-            SmallString<128> Fixed = Bad;
-            Fixed.append(".Private");
-            D << FixItHint::CreateReplacement(ActiveModule->DefinitionLoc,
-                                              Fixed);
-          }
-          break;
-        }
-      }
-    }
-  }
+
+  // Private modules named as FooPrivate, Foo.Private or similar are likely a
+  // user error; provide warnings, notes and fixits to direct users to use
+  // Foo_Private instead.
+  SourceLocation StartLoc =
+      SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID());
+  StringRef MapFileName(ModuleMapFile->getName());
+  if (Map.HeaderInfo.getHeaderSearchOpts().ImplicitModuleMaps &&
+      !Diags.isIgnored(diag::warn_mmap_mismatched_private_submodule,
+                       StartLoc) &&
+      !Diags.isIgnored(diag::warn_mmap_mismatched_private_module_name,
+                       StartLoc) &&
+      (MapFileName.endswith("module.private.modulemap") ||
+       MapFileName.endswith("module_private.map")))
+    diagnosePrivateModules(Map, Diags, ActiveModule);
 
   bool Done = false;
   do {

diff --git a/test/Modules/Inputs/implicit-private-canonical/A.framework/Headers/a.h b/test/Modules/Inputs/implicit-private-canonical/A.framework/Headers/a.h
new file mode 100644 (file)
index 0000000..8b4b198
--- /dev/null
+++ b/test/Modules/Inputs/implicit-private-canonical/A.framework/Headers/a.h
@@ -0,0 +1 @@
+extern int APUBLIC;

diff --git a/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h b/test/Modules/Inputs/implicit-private-canonical/A.framework/Headers/aprivate.h
similarity index 100%
rename from test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h
rename to test/Modules/Inputs/implicit-private-canonical/A.framework/Headers/aprivate.h

diff --git a/test/Modules/Inputs/implicit-private-canonical/A.framework/Modules/module.modulemap b/test/Modules/Inputs/implicit-private-canonical/A.framework/Modules/module.modulemap
new file mode 100644 (file)
index 0000000..95eabf9
--- /dev/null
+++ b/test/Modules/Inputs/implicit-private-canonical/A.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module A {
+  header "a.h"
+  export *
+}

diff --git a/test/Modules/Inputs/implicit-private-canonical/A.framework/Modules/module.private.modulemap b/test/Modules/Inputs/implicit-private-canonical/A.framework/Modules/module.private.modulemap
new file mode 100644 (file)
index 0000000..a7606f9
--- /dev/null
+++ b/test/Modules/Inputs/implicit-private-canonical/A.framework/Modules/module.private.modulemap
@@ -0,0 +1,4 @@
+framework module A_Private {
+  header "aprivate.h"
+  export *
+}

diff --git a/test/Modules/Inputs/implicit-private-canonical/A.framework/PrivateHeaders/aprivate.h b/test/Modules/Inputs/implicit-private-canonical/A.framework/PrivateHeaders/aprivate.h
new file mode 100644 (file)
index 0000000..760d901
--- /dev/null
+++ b/test/Modules/Inputs/implicit-private-canonical/A.framework/PrivateHeaders/aprivate.h
@@ -0,0 +1 @@
+extern int APRIVATE;

diff --git a/test/Modules/Inputs/implicit-private-with-different-name/A.framework/PrivateHeaders/aprivate.h b/test/Modules/Inputs/implicit-private-with-different-name/A.framework/PrivateHeaders/aprivate.h
new file mode 100644 (file)
index 0000000..760d901
--- /dev/null
+++ b/test/Modules/Inputs/implicit-private-with-different-name/A.framework/PrivateHeaders/aprivate.h
@@ -0,0 +1 @@
+extern int APRIVATE;

diff --git a/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Headers/a.h b/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Headers/a.h
new file mode 100644 (file)
index 0000000..8b4b198
--- /dev/null
+++ b/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Headers/a.h
@@ -0,0 +1 @@
+extern int APUBLIC;

diff --git a/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Headers/aprivate.h b/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Headers/aprivate.h
new file mode 100644 (file)
index 0000000..760d901
--- /dev/null
+++ b/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Headers/aprivate.h
@@ -0,0 +1 @@
+extern int APRIVATE;

diff --git a/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Modules/module.modulemap b/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Modules/module.modulemap
new file mode 100644 (file)
index 0000000..95eabf9
--- /dev/null
+++ b/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module A {
+  header "a.h"
+  export *
+}

diff --git a/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Modules/module.private.modulemap b/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Modules/module.private.modulemap
new file mode 100644 (file)
index 0000000..4018296
--- /dev/null
+++ b/test/Modules/Inputs/implicit-private-with-submodule/A.framework/Modules/module.private.modulemap
@@ -0,0 +1,4 @@
+framework module A.Private {
+  header "aprivate.h"
+  export *
+}

diff --git a/test/Modules/Inputs/implicit-private-with-submodule/A.framework/PrivateHeaders/aprivate.h b/test/Modules/Inputs/implicit-private-with-submodule/A.framework/PrivateHeaders/aprivate.h
new file mode 100644 (file)
index 0000000..760d901
--- /dev/null
+++ b/test/Modules/Inputs/implicit-private-with-submodule/A.framework/PrivateHeaders/aprivate.h
@@ -0,0 +1 @@
+extern int APRIVATE;

diff --git a/test/Modules/add-remove-private.m b/test/Modules/add-remove-private.m
index dc73a096c8079d66a5c171d1d64d69ddb3673a1b..5e7a5a966a4da8fbd533a5719fcbd35821a5154e 100644 (file)
--- a/test/Modules/add-remove-private.m
+++ b/test/Modules/add-remove-private.m
@@ -4,7 +4,7 @@
 // RUN: cp -r %S/Inputs/AddRemovePrivate.framework %t/AddRemovePrivate.framework
 
 // Build with module.private.modulemap
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -verify -DP
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -verify -DP -Wno-private-module
 // RUN: cp %t.mcp/AddRemovePrivate.pcm %t/with.pcm
 
 // Build without module.private.modulemap
@@ -17,7 +17,7 @@
 
 // Build with module.private.modulemap (again)
 // RUN: cp %S/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap %t/AddRemovePrivate.framework/Modules/module.private.modulemap
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -verify -DP
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -verify -DP -Wno-private-module
 // RUN: not diff %t.mcp/AddRemovePrivate.pcm %t/without.pcm
 
 // expected-no-diagnostics

diff --git a/test/Modules/auto-module-import.m b/test/Modules/auto-module-import.m
index 9a34c92eab2ff63c515c84bdb7e7a65a502c61fa..f6127adcbd8952007d77e5e979f26c7ad53c8e19 100644 (file)
--- a/test/Modules/auto-module-import.m
+++ b/test/Modules/auto-module-import.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -DERRORS
-// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify
-// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -xobjective-c++ %s -verify
+// RUN: %clang_cc1 -Wauto-import -Wno-private-module -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -DERRORS
+// RUN: %clang_cc1 -Wauto-import -Wno-private-module -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify
+// RUN: %clang_cc1 -Wauto-import -Wno-private-module -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -xobjective-c++ %s -verify
 // 
 // Test both with and without the declarations that refer to unimported
 // entities. For error recovery, those cases implicitly trigger an import.

diff --git a/test/Modules/global_index.m b/test/Modules/global_index.m
index 64a70f2a43ff3d3a02f3a4e02483f4ef1b51963a..e94c69ac3c1cb59a31de725d0e0a2c8443942d00 100644 (file)
--- a/test/Modules/global_index.m
+++ b/test/Modules/global_index.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t
 // Run without global module index
-// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -fno-modules-global-index -F %S/Inputs %s -verify
+// RUN: %clang_cc1 -Wauto-import -Wno-private-module -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -fno-modules-global-index -F %S/Inputs %s -verify
 // RUN: ls %t|not grep modules.idx
 // Run and create the global module index
-// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify
+// RUN: %clang_cc1 -Wauto-import -Wno-private-module -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify
 // RUN: ls %t|grep modules.idx
 // Run and use the global module index
-// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -print-stats 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -Wauto-import -Wno-private-module -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -print-stats 2>&1 | FileCheck %s
 
 // expected-no-diagnostics
 @import DependsOnModule;

diff --git a/test/Modules/implicit-private-canonical.m b/test/Modules/implicit-private-canonical.m
new file mode 100644 (file)
index 0000000..96b6c4a
--- /dev/null
+++ b/test/Modules/implicit-private-canonical.m
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// Build PCH using A, with adjacent private module APrivate, which winds up being implicitly referenced
+// 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 -DNO_AT_IMPORT
+// Use the PCH with no explicit way to resolve APrivate, still pick it up by automatic second-chance search for "A" with "Private" appended
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-canonical -include-pch %t-A.pch %s -fsyntax-only -Wprivate-module -DNO_AT_IMPORT
+
+// 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 -DUSE_AT_IMPORT_PRIV
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-canonical -include-pch %t-A.pch %s -fsyntax-only -Wprivate-module -DUSE_AT_IMPORT_PRIV
+
+// 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 -DUSE_AT_IMPORT_BOTH
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-canonical -include-pch %t-A.pch %s -fsyntax-only -Wprivate-module -DUSE_AT_IMPORT_BOTH
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+#ifdef NO_AT_IMPORT
+#import "A/aprivate.h"
+#endif
+
+#ifdef USE_AT_IMPORT_PRIV
+@import A_Private;
+#endif
+
+#ifdef USE_AT_IMPORT_BOTH
+@import A;
+@import A_Private;
+#endif
+
+const int *y = &APRIVATE;
+
+#endif

diff --git a/test/Modules/implicit-private-with-different-name.m b/test/Modules/implicit-private-with-different-name.m
index c09d3979c3e1dfcc518822f081232640c2de4ae1..7ee84539bf3aa1236268317ea3220722ec2cd88a 100644 (file)
--- a/test/Modules/implicit-private-with-different-name.m
+++ b/test/Modules/implicit-private-with-different-name.m
@@ -1,17 +1,17 @@
 // RUN: rm -rf %t
 
 // Build PCH using A, with adjacent private module APrivate, which winds up being implicitly referenced
-// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -emit-pch -o %t-A.pch %s
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -emit-pch -o %t-A.pch %s -Wprivate-module
 
 // Use the PCH with no explicit way to resolve APrivate, still pick it up by automatic second-chance search for "A" with "Private" appended
-// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only -Wprivate-module
 
 // Check the fixit
-// RUN: %clang_cc1  -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only -fdiagnostics-parseable-fixits -Wprivate-module %s 2>&1 | FileCheck %s
 
-// expected-warning@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{top-level module 'APrivate' in private module map, expected a submodule of 'A'}}
-// expected-note@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{make 'APrivate' a submodule of 'A' to ensure it can be found by name}}
-// CHECK: fix-it:"{{.*}}module.private.modulemap":{1:18-1:26}:"A.Private"
+// expected-warning@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{expected canonical name for private module 'APrivate'}}
+// expected-note@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{rename 'APrivate' to ensure it can be found by name}}
+// CHECK: fix-it:"{{.*}}module.private.modulemap":{1:18-1:26}:"A_Private"
 
 #ifndef HEADER
 #define HEADER

diff --git a/test/Modules/implicit-private-with-submodule.m b/test/Modules/implicit-private-with-submodule.m
new file mode 100644 (file)
index 0000000..1779341
--- /dev/null
+++ b/test/Modules/implicit-private-with-submodule.m
@@ -0,0 +1,36 @@
+// RUN: rm -rf %t
+// Build PCH using A, with private submodule A.Private
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-submodule -emit-pch -o %t-A.pch %s -DNO_AT_IMPORT
+
+// RUN: rm -rf %t
+// Build PCH using A, with private submodule A.Private, check the fixit
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-submodule -emit-pch -o %t-A.pch %s -fdiagnostics-parseable-fixits -DNO_AT_IMPORT 2>&1 | FileCheck %s
+
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-submodule -emit-pch -o %t-A.pch %s -DUSE_AT_IMPORT_PRIV
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-submodule -emit-pch -o %t-A.pch %s -DUSE_AT_IMPORT_BOTH
+
+// expected-warning@Inputs/implicit-private-with-submodule/A.framework/Modules/module.private.modulemap:1{{private submodule 'A.Private' in private module map, expected top-level module}}
+// expected-note@Inputs/implicit-private-with-submodule/A.framework/Modules/module.private.modulemap:1{{rename 'A.Private' to ensure it can be found by name}}
+// CHECK: fix-it:"{{.*}}module.private.modulemap":{1:20-1:27}:"A_Private"
+
+#ifndef HEADER
+#define HEADER
+
+#ifdef NO_AT_IMPORT
+#import "A/aprivate.h"
+#endif
+
+#ifdef USE_AT_IMPORT_PRIV
+@import A.Private;
+#endif
+
+#ifdef USE_AT_IMPORT_BOTH
+@import A;
+@import A.Private;
+#endif
+
+const int *y = &APRIVATE;
+
+#endif

diff --git a/test/Modules/modulemap-locations.m b/test/Modules/modulemap-locations.m
index 3c80db582d1d27cb717ee0276194778d0c7af587..c99bb14dc718b6b9f47acefbd431caa81a6ef34a 100644 (file)
--- a/test/Modules/modulemap-locations.m
+++ b/test/Modules/modulemap-locations.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/ModuleMapLocations/Module_ModuleMap -I %S/Inputs/ModuleMapLocations/Both -F %S/Inputs/ModuleMapLocations -I %S/Inputs/ModuleMapLocations -F %S/Inputs -x objective-c -fsyntax-only %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/ModuleMapLocations/Module_ModuleMap -I %S/Inputs/ModuleMapLocations/Both -F %S/Inputs/ModuleMapLocations -I %S/Inputs/ModuleMapLocations -F %S/Inputs -x objective-c -fsyntax-only %s -verify -Wno-private-module
 
 // regular
 @import module_modulemap;

diff --git a/test/Modules/prune.m b/test/Modules/prune.m
index 58992f9c006d210341b5adcc1523898cad3e6f7f..97a2fd7d0dcc7aa47b0403ddd26bb924443a90a1 100644 (file)
--- a/test/Modules/prune.m
+++ b/test/Modules/prune.m
@@ -8,8 +8,8 @@
 // Clear out the module cache
 // RUN: rm -rf %t
 // Run Clang twice so we end up creating the timestamp file (the second time).
-// RUN: %clang_cc1 -DIMPORT_DEPENDS_ON_MODULE -fmodules-ignore-macro=DIMPORT_DEPENDS_ON_MODULE -fmodules -fimplicit-module-maps -F %S/Inputs -fmodules-cache-path=%t %s -verify
-// RUN: %clang_cc1 -DIMPORT_DEPENDS_ON_MODULE -fmodules-ignore-macro=DIMPORT_DEPENDS_ON_MODULE -fmodules -fimplicit-module-maps -F %S/Inputs -fmodules-cache-path=%t %s -verify
+// RUN: %clang_cc1 -DIMPORT_DEPENDS_ON_MODULE -Wno-private-module -fmodules-ignore-macro=DIMPORT_DEPENDS_ON_MODULE -fmodules -fimplicit-module-maps -F %S/Inputs -fmodules-cache-path=%t %s -verify
+// RUN: %clang_cc1 -DIMPORT_DEPENDS_ON_MODULE -Wno-private-module -fmodules-ignore-macro=DIMPORT_DEPENDS_ON_MODULE -fmodules -fimplicit-module-maps -F %S/Inputs -fmodules-cache-path=%t %s -verify
 // RUN: ls %t | grep modules.timestamp
 // RUN: ls -R %t | grep ^Module.*pcm
 // RUN: ls -R %t | grep DependsOnModule.*pcm
@@ -17,7 +17,7 @@
 // Set the timestamp back more than two days. We should try to prune,
 // but nothing gets pruned because the module files are new enough.
 // RUN: touch -m -a -t 201101010000 %t/modules.timestamp 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -F %S/Inputs -fmodules-cache-path=%t -fmodules -fmodules-prune-interval=172800 -fmodules-prune-after=345600 %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -Wno-private-module -F %S/Inputs -fmodules-cache-path=%t -fmodules -fmodules-prune-interval=172800 -fmodules-prune-after=345600 %s -verify
 // RUN: ls %t | grep modules.timestamp
 // RUN: ls -R %t | grep ^Module.*pcm
 // RUN: ls -R %t | grep DependsOnModule.*pcm
@@ -26,7 +26,7 @@
 // This shouldn't prune anything, because the timestamp has been updated, so
 // the pruning mechanism won't fire.
 // RUN: find %t -name DependsOnModule*.pcm | sed -e 's/\\/\//g' | xargs touch -a -t 201101010000
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -F %S/Inputs -fmodules-cache-path=%t -fmodules -fmodules-prune-interval=172800 -fmodules-prune-after=345600 %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -F %S/Inputs -Wno-private-module -fmodules-cache-path=%t -fmodules -fmodules-prune-interval=172800 -fmodules-prune-after=345600 %s -verify
 // RUN: ls %t | grep modules.timestamp
 // RUN: ls -R %t | grep ^Module.*pcm
 // RUN: ls -R %t | grep DependsOnModule.*pcm
@@ -35,7 +35,7 @@
 // This should trigger pruning, which will remove DependsOnModule but not Module.
 // RUN: touch -m -a -t 201101010000 %t/modules.timestamp 
 // RUN: find %t -name DependsOnModule*.pcm | sed -e 's/\\/\//g' | xargs touch -a -t 201101010000
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -F %S/Inputs -fmodules-cache-path=%t -fmodules -fmodules-prune-interval=172800 -fmodules-prune-after=345600 %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -F %S/Inputs -Wno-private-module -fmodules-cache-path=%t -fmodules -fmodules-prune-interval=172800 -fmodules-prune-after=345600 %s -verify
 // RUN: ls %t | grep modules.timestamp
 // RUN: ls -R %t | grep ^Module.*pcm
 // RUN: ls -R %t | not grep DependsOnModule.*pcm

diff --git a/test/Modules/redefinition-c-tagtypes.m b/test/Modules/redefinition-c-tagtypes.m
index a01f11bd74c8b7e1842a94890eaf8f2ba0ce4d7e..eb469e00e16ff8b851e64c5a1139895568e7b118 100644 (file)
--- a/test/Modules/redefinition-c-tagtypes.m
+++ b/test/Modules/redefinition-c-tagtypes.m
@@ -1,8 +1,8 @@
 // RUN: rm -rf %t.cache
 // RUN: %clang_cc1 -fsyntax-only %s -fmodules -fmodules-cache-path=%t.cache \
-// RUN:   -fimplicit-module-maps -F%S/Inputs -verify
+// RUN:   -fimplicit-module-maps -Wno-private-module -F%S/Inputs -verify
 // RUN: %clang_cc1 -fsyntax-only %s -fmodules -fmodules-cache-path=%t.cache \
-// RUN:   -fimplicit-module-maps -F%S/Inputs -DCHANGE_TAGS -verify
+// RUN:   -fimplicit-module-maps -Wno-private-module -F%S/Inputs -DCHANGE_TAGS -verify
 #include "F/F.h"
 
 #ifndef CHANGE_TAGS

diff --git a/test/Modules/requires-coroutines.mm b/test/Modules/requires-coroutines.mm
index 8e25a3c575211ab6637082cb1532103219a525f3..4e9c9d19cd8009f7a8c3add18d7ecd66633a7fb2 100644 (file)
--- a/test/Modules/requires-coroutines.mm
+++ b/test/Modules/requires-coroutines.mm
@@ -1,6 +1,6 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -I %S/Inputs/DependsOnModule.framework %s -verify
-// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -I %S/Inputs/DependsOnModule.framework %s -verify -fcoroutines-ts -DCOROUTINES
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -Wno-private-module -F %S/Inputs -I %S/Inputs/DependsOnModule.framework %s -verify
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -Wno-private-module -F %S/Inputs -I %S/Inputs/DependsOnModule.framework %s -verify -fcoroutines-ts -DCOROUTINES
 
 #ifdef COROUTINES
 @import DependsOnModule.Coroutines;

diff --git a/test/Modules/requires.m b/test/Modules/requires.m
index d61de6bd48d0ecc346ac59cb6e6a70a9ecf6a46f..4a83d0c2b0ec1271e61f8e253115498a002570ac 100644 (file)
--- a/test/Modules/requires.m
+++ b/test/Modules/requires.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -I %S/Inputs %s -verify -fmodule-feature custom_req1
+// RUN: %clang_cc1 -Wauto-import -Wno-private-module -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -I %S/Inputs %s -verify -fmodule-feature custom_req1
 
 // expected-error@DependsOnModule.framework/module.map:7 {{module 'DependsOnModule.CXX' requires feature 'cplusplus'}}
 @import DependsOnModule.CXX; // expected-note {{module imported here}}

diff --git a/test/Modules/requires.mm b/test/Modules/requires.mm
index f90622ece842afaf56a448373fc434e01a43f939..b4237cbd7874e80b998a28835c284cf8cacc63e1 100644 (file)
--- a/test/Modules/requires.mm
+++ b/test/Modules/requires.mm
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -I %S/Inputs/DependsOnModule.framework %s -verify
+// RUN: %clang_cc1 -Wauto-import -Wno-private-module -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -I %S/Inputs/DependsOnModule.framework %s -verify
 
 @import DependsOnModule.CXX;
 // expected-error@module.map:11 {{module 'DependsOnModule.NotCXX' is incompatible with feature 'cplusplus'}}

diff --git a/test/Modules/subframework-from-intermediate-path.m b/test/Modules/subframework-from-intermediate-path.m
index 394cc45f2f60c550dbb049bb9944ebe2fa7bf8aa..1543861ec4c8f953a0dc4a6db5a70d454c5360e1 100644 (file)
--- a/test/Modules/subframework-from-intermediate-path.m
+++ b/test/Modules/subframework-from-intermediate-path.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -F %S/Inputs/DependsOnModule.framework/Frameworks %s -verify
+// RUN: %clang_cc1 -Wno-private-module -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -F %S/Inputs/DependsOnModule.framework/Frameworks %s -verify
 
 @import DependsOnModule;
 @import SubFramework; // expected-error{{module 'SubFramework' not found}}

diff --git a/test/Modules/subframeworks.m b/test/Modules/subframeworks.m
index 21081843d78cb616e9fdb20e370d1072f64c56b5..ce35415717d2cc3f4742073b48ee6ff6924bd1fa 100644 (file)
--- a/test/Modules/subframeworks.m
+++ b/test/Modules/subframeworks.m
@@ -1,6 +1,6 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -F %S/Inputs/DependsOnModule.framework/Frameworks %s -verify
-// RUN: %clang_cc1 -x objective-c++ -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -F %S/Inputs/DependsOnModule.framework/Frameworks %s -verify
+// RUN: %clang_cc1 -Wauto-import -Wno-private-module -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -F %S/Inputs/DependsOnModule.framework/Frameworks %s -verify
+// RUN: %clang_cc1 -x objective-c++ -Wauto-import -Wno-private-module -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -F %S/Inputs/DependsOnModule.framework/Frameworks %s -verify
 
 @import DependsOnModule;