[modules] Restrict the module use-declaration to only appear in top-level

modules, and allow sub-modules of a module with a use-declaration to make use
of the nominated modules.


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

8 files changed, 61 insertions, 34 deletions

diff --git a/docs/Modules.rst b/docs/Modules.rst
index 1d4c1f46bc..9999106914 100644
--- a/docs/Modules.rst
+++ b/docs/Modules.rst
@@ -429,7 +429,7 @@ tls
   A specific target feature (e.g., ``sse4``, ``avx``, ``neon``) is available.
 
 
-**Example**: The ``std`` module can be extended to also include C++ and C++11 headers using a *requires-declaration*:
+**Example:** The ``std`` module can be extended to also include C++ and C++11 headers using a *requires-declaration*:
 
 .. parsed-literal::
 
@@ -470,11 +470,16 @@ A header with the ``umbrella`` specifier is called an umbrella header. An umbrel
 
 A header with the ``private`` specifier may not be included from outside the module itself.
 
-A header with the ``textual`` specifier will not be included when the module is built, and will be textually included if it is named by a ``#include`` directive. However, it is considered to be part of the module for the purpose of checking *use-declaration*\s.
+A header with the ``textual`` specifier will not be compiled when the module is
+built, and will be textually included if it is named by a ``#include``
+directive. However, it is considered to be part of the module for the purpose
+of checking *use-declaration*\s, and must still be a lexically-valid header
+file. In the future, we intend to pre-tokenize such headers and include the
+token sequence within the prebuilt module representation.
 
 A header with the ``exclude`` specifier is excluded from the module. It will not be included when the module is built, nor will it be considered to be part of the module, even if an ``umbrella`` header or directory would otherwise make it part of the module.
 
-**Example**: The C header ``assert.h`` is an excellent candidate for a textual header, because it is meant to be included multiple times (possibly with different ``NDEBUG`` settings). However, declarations within it should typically be split into a separate modular header.
+**Example:** The C header ``assert.h`` is an excellent candidate for a textual header, because it is meant to be included multiple times (possibly with different ``NDEBUG`` settings). However, declarations within it should typically be split into a separate modular header.
 
 .. parsed-literal::
 
@@ -536,7 +541,7 @@ For each header included by the umbrella header or in the umbrella directory tha
 * Contain a single *header-declaration* naming that header
 * Contain a single *export-declaration* ``export *``, if the \ *inferred-submodule-declaration* contains the \ *inferred-submodule-member* ``export *``
 
-**Example**: If the subdirectory "MyLib" contains the headers ``A.h`` and ``B.h``, then the following module map:
+**Example:** If the subdirectory "MyLib" contains the headers ``A.h`` and ``B.h``, then the following module map:
 
 .. parsed-literal::
 
@@ -579,7 +584,7 @@ An *export-declaration* specifies which imported modules will automatically be r
 
 The *export-declaration* names a module or a set of modules that will be re-exported to any translation unit that imports the enclosing module. Each imported module that matches the *wildcard-module-id* up to, but not including, the first ``*`` will be re-exported.
 
-**Example**:: In the following example, importing ``MyLib.Derived`` also provides the API for ``MyLib.Base``:
+**Example:** In the following example, importing ``MyLib.Derived`` also provides the API for ``MyLib.Base``:
 
 .. parsed-literal::
 
@@ -623,14 +628,16 @@ Note that, if ``Derived.h`` includes ``Base.h``, one can simply use a wildcard e
 
 Use declaration
 ~~~~~~~~~~~~~~~
-A *use-declaration* specifies one of the other modules that the module is allowed to use. An import or include not matching one of these is rejected when the option *-fmodules-decluse*.
+A *use-declaration* specifies another module that the current top-level module
+intends to use. When the option *-fmodules-decluse* is specified, a module can
+only use other modules that are explicitly specified in this way.
 
 .. parsed-literal::
 
   *use-declaration*:
     ``use`` *module-id*
 
-**Example**:: In the following example, use of A from C is not declared, so will trigger a warning.
+**Example:** In the following example, use of A from C is not declared, so will trigger a warning.
 
 .. parsed-literal::
 
@@ -647,7 +654,9 @@ A *use-declaration* specifies one of the other modules that the module is allowe
     use B
   }
 
-When compiling a source file that implements a module, use the option ``-fmodule-name=module-id`` to indicate that the source file is logically part of that module.
+When compiling a source file that implements a module, use the option
+``-fmodule-name=module-id`` to indicate that the source file is logically part
+of that module.
 
 The compiler at present only applies restrictions to the module directly being built.
 
diff --git a/include/clang/Basic/DiagnosticLexKinds.td b/include/clang/Basic/DiagnosticLexKinds.td
index 3fa9bcf419..6eaf423a77 100644
--- a/include/clang/Basic/DiagnosticLexKinds.td
+++ b/include/clang/Basic/DiagnosticLexKinds.td
@@ -579,7 +579,9 @@ def err_mmap_module_id : Error<
 def err_mmap_expected_library_name : Error<
   "expected %select{library|framework}0 name as a string">;
 def err_mmap_config_macro_submodule : Error<
-  "configuration macros are only allowed on top-level modules">;
+  "configuration macros are only allowed in top-level modules">;
+def err_mmap_use_decl_submodule : Error<
+  "use declarations are only allowed in top-level modules">;
 def err_mmap_expected_config_macro : Error<
   "expected configuration macro name after ','">;
 def err_mmap_expected_conflicts_comma : Error<
diff --git a/include/clang/Basic/Module.h b/include/clang/Basic/Module.h
index e3953a4571..a976601c51 100644
--- a/include/clang/Basic/Module.h
+++ b/include/clang/Basic/Module.h
@@ -399,6 +399,10 @@ public:
   /// \brief The top-level headers associated with this module.
   ArrayRef<const FileEntry *> getTopHeaders(FileManager &FileMgr);
 
+  /// \brief Determine whether this module has declared its intention to
+  /// directly use another module.
+  bool directlyUses(const Module *Requested) const;
+
   /// \brief Add the given feature requirement to the list of features
   /// required by this module.
   ///
diff --git a/lib/Basic/Module.cpp b/lib/Basic/Module.cpp
index e7e37ced80..5fad1a9b22 100644
--- a/lib/Basic/Module.cpp
+++ b/lib/Basic/Module.cpp
@@ -158,6 +158,19 @@ ArrayRef<const FileEntry *> Module::getTopHeaders(FileManager &FileMgr) {
   return llvm::makeArrayRef(TopHeaders.begin(), TopHeaders.end());
 }
 
+bool Module::directlyUses(const Module *Requested) const {
+  auto *Top = getTopLevelModule();
+
+  // A top-level module implicitly uses itself.
+  if (Requested->isSubModuleOf(Top))
+    return true;
+
+  for (auto *Use : Top->DirectUses)
+    if (Requested->isSubModuleOf(Use))
+      return true;
+  return false;
+}
+
 void Module::addRequirement(StringRef Feature, bool RequiredState,
                             const LangOptions &LangOpts,
                             const TargetInfo &Target) {
diff --git a/lib/Lex/ModuleMap.cpp b/lib/Lex/ModuleMap.cpp
index 1134cce84f..a4f1c05dc1 100644
--- a/lib/Lex/ModuleMap.cpp
+++ b/lib/Lex/ModuleMap.cpp
@@ -205,16 +205,6 @@ ModuleMap::findHeaderInUmbrellaDirs(const FileEntry *File,
   return KnownHeader();
 }
 
-// Returns true if RequestingModule directly uses RequestedModule.
-static bool directlyUses(const Module *RequestingModule,
-                         const Module *RequestedModule) {
-  for (const Module* DirectUse : RequestingModule->DirectUses) {
-    if (RequestedModule->isSubModuleOf(DirectUse))
-      return true;
-  }
-  return false;
-}
-
 static bool violatesPrivateInclude(Module *RequestingModule,
                                    const FileEntry *IncFileEnt,
                                    ModuleMap::ModuleHeaderRole Role,
@@ -238,6 +228,9 @@ static bool violatesPrivateInclude(Module *RequestingModule,
   }
 #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;
 }
 
@@ -279,7 +272,7 @@ void ModuleMap::diagnoseHeaderInclusion(Module *RequestingModule,
       // If uses need to be specified explicitly, we are only allowed to return
       // modules that are explicitly used by the requesting module.
       if (RequestingModule && LangOpts.ModulesDeclUse &&
-          !directlyUses(RequestingModule, Header.getModule())) {
+          !RequestingModule->directlyUses(Header.getModule())) {
         NotUsed = Header.getModule();
         continue;
       }
@@ -368,7 +361,7 @@ ModuleMap::findModuleForHeader(const FileEntry *File,
       // If uses need to be specified explicitly, we are only allowed to return
       // modules that are explicitly used by the requesting module.
       if (RequestingModule && LangOpts.ModulesDeclUse &&
-          !directlyUses(RequestingModule, I->getModule()))
+          !RequestingModule->directlyUses(I->getModule()))
         continue;
 
       if (!Result || isBetterKnownHeader(*I, Result))
@@ -1918,18 +1911,21 @@ void ModuleMapParser::parseExportDecl() {
   ActiveModule->UnresolvedExports.push_back(Unresolved);
 }
 
-/// \brief Parse a module uses declaration.
+/// \brief Parse a module use declaration.
 ///
-///   uses-declaration:
-///     'uses' wildcard-module-id
+///   use-declaration:
+///     'use' wildcard-module-id
 void ModuleMapParser::parseUseDecl() {
   assert(Tok.is(MMToken::UseKeyword));
-  consumeToken();
+  auto KWLoc = consumeToken();
   // Parse the module-id.
   ModuleId ParsedModuleId;
   parseModuleId(ParsedModuleId);
 
-  ActiveModule->UnresolvedDirectUses.push_back(ParsedModuleId);
+  if (ActiveModule->Parent)
+    Diags.Report(KWLoc, diag::err_mmap_use_decl_submodule);
+  else
+    ActiveModule->UnresolvedDirectUses.push_back(ParsedModuleId);
 }
 
 /// \brief Parse a link declaration.
diff --git a/test/Modules/Inputs/declare-use/module.map b/test/Modules/Inputs/declare-use/module.map
index a74a7ae04a..2dad0d061c 100644
--- a/test/Modules/Inputs/declare-use/module.map
+++ b/test/Modules/Inputs/declare-use/module.map
@@ -3,7 +3,7 @@ module XA {
 }
 
 module XB {
-  header "b.h"
+  module B { header "b.h" }
 }
 
 module XC {
@@ -43,7 +43,7 @@ module XG {
 }
 
 module XH {
-  header "h.h"
+  module H { header "h.h" }
   header "h1.h"
   header "s.h"
   use XC
diff --git a/test/Modules/Inputs/diagnostics-aux.modulemap b/test/Modules/Inputs/diagnostics-aux.modulemap
new file mode 100644
index 0000000000..d067d04d3d
--- /dev/null
+++ b/test/Modules/Inputs/diagnostics-aux.modulemap
@@ -0,0 +1 @@
+module foo {}
diff --git a/test/Modules/diagnostics.modulemap b/test/Modules/diagnostics.modulemap
index 14eb408aeb..aef094d3bf 100644
--- a/test/Modules/diagnostics.modulemap
+++ b/test/Modules/diagnostics.modulemap
@@ -1,12 +1,14 @@
-// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fmodule-map-file=%s -fsyntax-only -x c++ /dev/null 2>&1
-//
-// RUN: cp %s %t-duplicate.modulemap
-// RUN: not %clang_cc1 -fmodules -fmodules-cache-path=%t -fmodule-map-file=%s -fmodule-map-file=%t-duplicate.modulemap -fsyntax-only -x c++ /dev/null 2>&1 | FileCheck --check-prefix=CHECK-DUPLICATE %s
+// RUN: not %clang_cc1 -fmodules -fmodules-cache-path=%t -fmodule-map-file=%S/Inputs/diagnostics-aux.modulemap -fmodule-map-file=%s -fsyntax-only -x c++ /dev/null 2>&1 | FileCheck %s
 
 // PR22299: Ensure we can produce diagnostics for duplicate modules from -fmodule-map-file=.
 //
-// CHECK-DUPLICATE: duplicate.modulemap:[[@LINE+2]]:8: error: redefinition of module 'foo'
-// CHECK-DUPLICATE: diagnostics.modulemap:[[@LINE+1]]:8: note: previously defined here
+// CHECK: diagnostics.modulemap:[[@LINE+2]]:8: error: redefinition of module 'foo'
+// CHECK: diagnostics-aux.modulemap:1:8: note: previously defined here
 module foo {}
 
 //* Check that we accept BCPL comments properly, not just as an extension. */
+
+module bad_use {
+  // CHECK: diagnostics.modulemap:[[@LINE+1]]:22: error: use declarations are only allowed in top-level modules
+  module submodule { use foo }
+}