[modules] PR20507: Avoid silent textual inclusion.

Summary:
If a module was unavailable (either a missing requirement on the module
being imported, or a missing file anywhere in the top-level module (and
not dominated by an unsatisfied `requires`)), we would silently treat
inclusions as textual. This would cause all manner of crazy and
confusing errors (and would also silently "work" sometimes, making the
problem difficult to track down).

I'm really not a fan of the `M->isAvailable(getLangOpts(), getTargetInfo(),
Requirement, MissingHeader)` function; it seems to do too many things at
once, but for now I've done things in a sort of awkward way.

The changes to test/Modules/Inputs/declare-use/module.map
were necessitated because the thing that was meant to be tested there
(introduced in r197805) was predicated on silently falling back to textual
inclusion, which we no longer do.

The changes to test/Modules/Inputs/macro-reexport/module.modulemap
are just an overlooked missing header that seems to have been missing since
this code was committed (r213922), which is now caught.

Reviewers: rsmith, benlangmuir, djasper

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D10423

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

Author: chisophugis

include/clang/Basic/DiagnosticCommonKinds.td

@@ -84,6 +84,10 @@ def err_module_not_built : Error<"could not build module '%0'">, DefaultFatal;
 def err_module_build_disabled: Error<
   "module '%0' is needed but has not been provided, and implicit use of module "
   "files is disabled">, DefaultFatal;
+def err_module_unavailable : Error<
+  "module '%0' %select{is incompatible with|requires}1 feature '%2'">;
+def err_module_header_missing : Error<
+  "%select{|umbrella }0header '%1' not found">;
 def err_module_lock_failure : Error<
   "could not acquire lock file for module '%0'">, DefaultFatal;
 def err_module_lock_timeout : Error<

include/clang/Basic/DiagnosticFrontendKinds.td

@@ -183,10 +183,6 @@ def err_no_submodule_suggest : Error<
   "no submodule named %0 in module '%1'; did you mean '%2'?">;
 def warn_missing_submodule : Warning<"missing submodule '%0'">,
   InGroup<IncompleteUmbrella>;
-def err_module_unavailable : Error<
-  "module '%0' %select{is incompatible with|requires}1 feature '%2'">;
-def err_module_header_missing : Error<
-  "%select{|umbrella }0header '%1' not found">;
 def err_module_cannot_create_includes : Error<
   "cannot create includes file for module %0: %1">;
 def warn_module_config_macro_undef : Warning<

include/clang/Basic/DiagnosticLexKinds.td

@@ -623,6 +623,8 @@ def warn_mmap_unknown_attribute : Warning<"unknown attribute '%0'">,
 def warn_auto_module_import : Warning<
   "treating #%select{include|import|include_next|__include_macros}0 as an "
   "import of module '%1'">, InGroup<AutoImport>, DefaultIgnore;
+def note_implicit_top_level_module_import_here : Note<
+  "submodule of top-level module '%0' implicitly imported here">;
 def warn_uncovered_module_header : Warning<
   "umbrella header for module '%0' does not include header '%1'">, 
   InGroup<IncompleteUmbrella>;

lib/Lex/ModuleMap.cpp

@@ -320,6 +320,10 @@ void ModuleMap::diagnoseHeaderInclusion(Module *RequestingModule,
 
 static bool isBetterKnownHeader(const ModuleMap::KnownHeader &New,
                                 const ModuleMap::KnownHeader &Old) {
+  // Prefer available modules.
+  if (New.getModule()->isAvailable() && !Old.getModule()->isAvailable())
+    return true;
+
   // Prefer a public header over a private header.
   if ((New.getRole() & ModuleMap::PrivateHeader) !=
       (Old.getRole() & ModuleMap::PrivateHeader))
@@ -349,9 +353,6 @@ ModuleMap::KnownHeader ModuleMap::findModuleForHeader(const FileEntry *File) {
       // Prefer a header from the current module over all others.
       if (H.getModule()->getTopLevelModule() == CompilingModule)
         return MakeResult(H);
-      // Cannot use a module if it is unavailable.
-      if (!H.getModule()->isAvailable())
-        continue;
       if (!Result || isBetterKnownHeader(H, Result))
         Result = H;
     }

lib/Lex/PPDirectives.cpp

@@ -1674,6 +1674,29 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
           getLangOpts().CurrentModule &&
       SuggestedModule.getModule()->getTopLevelModuleName() !=
           getLangOpts().ImplementationOfModule) {
+
+    // If this include corresponds to a module but that module is
+    // unavailable, diagnose the situation and bail out.
+    if (!SuggestedModule.getModule()->isAvailable()) {
+      clang::Module::Requirement Requirement;
+      clang::Module::UnresolvedHeaderDirective MissingHeader;
+      Module *M = SuggestedModule.getModule();
+      // Identify the cause.
+      (void)M->isAvailable(getLangOpts(), getTargetInfo(), Requirement,
+                           MissingHeader);
+      if (MissingHeader.FileNameLoc.isValid()) {
+        Diag(MissingHeader.FileNameLoc, diag::err_module_header_missing)
+            << MissingHeader.IsUmbrella << MissingHeader.FileName;
+      } else {
+        Diag(M->DefinitionLoc, diag::err_module_unavailable)
+            << M->getFullModuleName() << Requirement.second << Requirement.first;
+      }
+      Diag(FilenameTok.getLocation(),
+           diag::note_implicit_top_level_module_import_here)
+          << M->getTopLevelModuleName();
+      return;
+    }
+
     // Compute the module access path corresponding to this module.
     // FIXME: Should we have a second loadModule() overload to avoid this
     // extra lookup step?

test/Modules/Inputs/auto-import-unavailable/missing_header/not_missing.h

@@ -0,0 +1 @@
+// missing_header/not_missing.h

test/Modules/Inputs/auto-import-unavailable/missing_requirement.h

@@ -0,0 +1 @@
+// missing_requirement.h

test/Modules/Inputs/auto-import-unavailable/module.modulemap

@@ -0,0 +1,19 @@
+module missing_header {
+  module missing { header "missing_header/missing.h" }
+  module error_importing_this { header "missing_header/not_missing.h" }
+}
+
+module nonrequired_missing_header {
+  module unsatisfied_requires {
+    requires nonexistent_feature
+    header "nonrequired_missing_header/missing.h"
+  }
+  module fine_to_import {
+    header "nonrequired_missing_header/not_missing.h"
+  }
+}
+
+module missing_requirement {
+  requires nonexistent_feature
+  header "missing_requirement.h"
+}

test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/not_missing.h

@@ -0,0 +1 @@
+// nonrequired_missing_header/not_missing.h

test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/requires_feature_you_dont_have.h

@@ -0,0 +1 @@
+// nonrequired_missing_header/requires_feature_you_dont_have.h

test/Modules/Inputs/available-is-better/available-is-better.h

@@ -0,0 +1,2 @@
+#pragma once
+int available;

test/Modules/Inputs/available-is-better/module.modulemap

@@ -0,0 +1,17 @@
+// There is some order-dependence to how clang chooses modules, so make
+// sure that both the first and last modules here are ones that would
+// cause a test failure if they were picked.
+
+module unavailable_before {
+  requires nonexistent_feature
+  header "available-is-better.h"
+}
+
+module available {
+  header "available-is-better.h"
+}
+
+module unavailable_after {
+  requires nonexistent_feature
+  header "available-is-better.h"
+}

test/Modules/Inputs/declare-use/module.map

@@ -20,14 +20,12 @@ module XD {
 
 module XE {
   header "e.h"
-  header "unavailable.h"
   use XA
   use XB
 }
 
 module XF {
   header "f.h"
-  header "unavailable.h"
   use XA
   use XB
 }

test/Modules/Inputs/macro-reexport/module.modulemap

@@ -19,5 +19,4 @@ module e {
 }
 module f {
   module f1 { header "f1.h" export * }
-  module f2 { header "f2.h" export * }
 }

test/Modules/auto-import-unavailable.cpp

@@ -0,0 +1,47 @@
+// RUN: rm -rf %t
+// RUN: not %clang_cc1 -x c++ -Rmodule-build -DMISSING_HEADER -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/auto-import-unavailable %s 2>&1 | FileCheck %s --check-prefix=MISSING-HEADER
+// RUN: %clang_cc1 -x c++ -Rmodule-build -DNONREQUIRED_MISSING_HEADER -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/auto-import-unavailable %s 2>&1 | FileCheck %s --check-prefix=NONREQUIRED-MISSING-HEADER
+// RUN: not %clang_cc1 -x c++ -Rmodule-build -DMISSING_REQUIREMENT -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/auto-import-unavailable %s 2>&1 | FileCheck %s --check-prefix=MISSING-REQUIREMENT
+
+#ifdef MISSING_HEADER
+
+// Even if the header we ask for is not missing, if the top-level module
+// containing it has a missing header, then the whole top-level is
+// unavailable and we issue an error.
+
+// MISSING-HEADER: module.modulemap:2:27: error: header 'missing_header/missing.h' not found
+// MISSING-HEADER-DAG: auto-import-unavailable.cpp:[[@LINE+1]]:10: note: submodule of top-level module 'missing_header' implicitly imported here
+#include "missing_header/not_missing.h"
+
+// We should not attempt to build the module.
+// MISSING-HEADER-NOT: remark: building module
+
+#endif // #ifdef MISSING_HEADER
+
+
+#ifdef NONREQUIRED_MISSING_HEADER
+
+// However, if the missing header is dominated by an unsatisfied
+// `requires`, then that is acceptable.
+// This also tests that an unsatisfied `requires` elsewhere in the
+// top-level module doesn't affect an available module.
+
+// NONREQUIRED-MISSING-HEADER: auto-import-unavailable.cpp:[[@LINE+2]]:10: remark: building module 'nonrequired_missing_header'
+// NONREQUIRED-MISSING-HEADER: auto-import-unavailable.cpp:[[@LINE+1]]:10: remark: finished building module 'nonrequired_missing_header'
+#include "nonrequired_missing_header/not_missing.h"
+
+#endif // #ifdef NONREQUIRED_MISSING_HEADER
+
+
+#ifdef MISSING_REQUIREMENT
+
+// If the header is unavailable due to a missing requirement, an error
+// should be emitted if a user tries to include it.
+
+// MISSING-REQUIREMENT:module.modulemap:16:8: error: module 'missing_requirement' requires feature 'nonexistent_feature'
+// MISSING-REQUIREMENT: auto-import-unavailable.cpp:[[@LINE+1]]:10: note: submodule of top-level module 'missing_requirement' implicitly imported here
+#include "missing_requirement.h"
+
+// MISSING-REQUIREMENT-NOT: remark: building module
+
+#endif // #ifdef MISSING_REQUIREMENT

test/Modules/available-is-better.cpp

@@ -0,0 +1,5 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x c++ -Rmodule-build -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/available-is-better %s 2>&1 | FileCheck %s
+
+#include "available-is-better.h"
+// CHECK: remark: building module