Hi rsmith, benlangmuir, djasper,

This pretty naively follows the implementation suggested by Richard.

I'd like to switch these over to be fatal errors (since they are
essentially similar to err_pp_file_not_found), but I'm still investigating
some test failures that happen when I switched them over.

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.

http://reviews.llvm.org/D10423

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/DiagnosticFrontendKinds.td
  lib/Lex/ModuleMap.cpp
  lib/Lex/PPDirectives.cpp
  test/Modules/Inputs/declare-use/module.map
  test/Modules/Inputs/macro-reexport/module.modulemap
  test/Modules/missing-header.cpp

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
Index: include/clang/Basic/DiagnosticCommonKinds.td
===================================================================
--- include/clang/Basic/DiagnosticCommonKinds.td
+++ include/clang/Basic/DiagnosticCommonKinds.td
@@ -84,6 +84,10 @@
 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<
Index: include/clang/Basic/DiagnosticFrontendKinds.td
===================================================================
--- include/clang/Basic/DiagnosticFrontendKinds.td
+++ include/clang/Basic/DiagnosticFrontendKinds.td
@@ -173,10 +173,6 @@
   "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<
Index: lib/Lex/ModuleMap.cpp
===================================================================
--- lib/Lex/ModuleMap.cpp
+++ lib/Lex/ModuleMap.cpp
@@ -341,13 +341,9 @@
   if (Known != Headers.end()) {
     ModuleMap::KnownHeader Result;
     // Iterate over all modules that 'File' is part of to find the best fit.
-    for (KnownHeader &H : Known->second) {
-      // Cannot use a module if it is unavailable.
-      if (!H.getModule()->isAvailable())
-        continue;
+    for (KnownHeader &H : Known->second)
       if (!Result || isBetterKnownHeader(H, Result))
         Result = H;
-    }
     return MakeResult(Result);
   }
 
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1654,6 +1654,25 @@
     }
   }
 
+  // If this include corresponds to a module but that module is
+  // unavailable, diagnose the situation and bail out.
+  if (SuggestedModule && !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(MissingHeader.FileNameLoc, diag::err_module_unavailable)
+          << M->getFullModuleName() << Requirement.second << Requirement.first;
+    }
+    return;
+  }
+
   // Should we enter the source file? Set to false if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined or is a modular header
Index: test/Modules/Inputs/declare-use/module.map
===================================================================
--- test/Modules/Inputs/declare-use/module.map
+++ test/Modules/Inputs/declare-use/module.map
@@ -20,14 +20,12 @@
 
 module XE {
   header "e.h"
-  header "unavailable.h"
   use XA
   use XB
 }
 
 module XF {
   header "f.h"
-  header "unavailable.h"
   use XA
   use XB
 }
Index: test/Modules/Inputs/macro-reexport/module.modulemap
===================================================================
--- test/Modules/Inputs/macro-reexport/module.modulemap
+++ test/Modules/Inputs/macro-reexport/module.modulemap
@@ -19,5 +19,4 @@
 }
 module f {
   module f1 { header "f1.h" export * }
-  module f2 { header "f2.h" export * }
 }
Index: test/Modules/missing-header.cpp
===================================================================
--- /dev/null
+++ test/Modules/missing-header.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: not %clang_cc1 -x c++ -fmodules -fmodules-cache-path=%t -I %S/Inputs/submodules %s 2>&1 | FileCheck %s
+
+// 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.
+
+#include "not_missing.h"
+
+// CHECK: module.map:15:27: error: header 'missing.h' not found
+// We should not attempt to build the module.
+// CHECK-NOT: could not build module 'missing_headers'
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to