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

[Ben Langmuir]

I tried this patch out and found a few issues I've commented on inline.  It also exposes the two issues below:

1. This patch exposes a compatibility issue with our 'Darwin' module map file (the module map that covers most of /usr/include on Darwin platforms).

  error: module 'Darwin.C.excluded' requires feature 'excluded'

This "excluded" submodule was used as a hack for "assert.h" before the explicit support for "exclude" and "textual" headers were added to clang.  After this patch, it won't be possible to include assert.h on Darwin, because the module it is in is missing a requirement.  I think it would be very bad to break compatibility here, so I propose we add a compatibility hack to module map parsing that treats a module with a the requirement "excluded" as if all its headers were declared with "exclude".  What do you think?  I'm happy to implement this.

2. If a require decl is written *after* a header decl in a module, that header still gets treated as required.  We should fix that before this is committed.  E.g.



  module Top {
    module A { header "foo.h" requires nope }
    module B { requires nope header "bar.h" }
  }

We'll complain about missing foo.h, but not about bar.h.


================
Comment at: lib/Lex/PPDirectives.cpp:1671
@@ +1670,3 @@
+      Diag(MissingHeader.FileNameLoc, diag::err_module_unavailable)
+          << M->getFullModuleName() << Requirement.second << Requirement.first;
+    }
----------------
1. This uses MissingHeader.FileNameLoc, which we just proved is invalid :-)
2. We should also show diag::note_implicit_top_level_module_import_here in this diagnostic.

================
Comment at: test/Modules/missing-header.cpp:13
@@ +12,2 @@
+// We should not attempt to build the module.
+// CHECK-NOT: could not build module 'missing_headers'
----------------
We should have a test for including a header from a module with a missing requirement.  And one where a missing header from a different submodule is okay because the submodule is missing a requirement.

http://reviews.llvm.org/D10423

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/