[PATCH] D10423: [modules] PR20507: Avoid silent textual inclusion.

[Richard Smith]

rsmith accepted this revision.
This revision is now accepted and ready to land.

================
Comment at: lib/Lex/PPDirectives.cpp:1680-1698
@@ +1679,21 @@
+    // 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;
+    }
+
----------------
silvas wrote:
> rsmith wrote:
> > The call to `loadModule` a few lines below already does this. Is this change necessary?
> I think it is. We have more information available here and can diagnose the situation better. Ideally the call to loadModule here would take a `Module *` and could assert the module to be available.
OK, that sounds like a good general direction. I don't like the duplication here, but if the plan is to eventually remove it, I'm OK with that.

I'm still concerned about the incomplete umbrella header case, but that really seems like a fundamental design problem with implicit module generation for incomplete umbrella headers -- if we can't build the umbrella header, we can't tell whether a file is part of it, so if the umbrella header module is unavailable, we have to either reject all inclusions of headers in its directory or silently ignore the umbrella header (and silently ignoring modules is exactly what you're preventing with this change).


http://reviews.llvm.org/D10423