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

Richard Smith richard at metafoo.co.uk
Wed Jul 8 14:05:10 PDT 2015


rsmith added inline comments.

================
Comment at: lib/Lex/PPDirectives.cpp:1665
@@ +1664,3 @@
+  // unavailable, diagnose the situation and bail out.
+  if (SuggestedModule && !SuggestedModule.getModule()->isAvailable()) {
+    clang::Module::Requirement Requirement;
----------------
I think this whole block should be moved down to line 1761 or so:

 1. The code currently ensures that it always calls InclusionDirective on the callback object for every `#include`, even if that include fails.
 2. We don't yet know that the file is actually part of `SuggestedModule`; it could be in the directory of an umbrella header whose module is unavailable, but it might not be part of that module.
 3. If we somehow got a suggested module but no file, we should not produce additional spurious diagnostics beyond the "file not found" diagnostic we already produced.

================
Comment at: test/Modules/Inputs/auto-import-unavailable/module.modulemap:9
@@ +8,3 @@
+    requires nonexistent_feature
+    header "nonrequired_missing_header/missing.h"
+  }
----------------
Please add some content to the empty files (even if just a comment) or this test will misbehave on content-addressed file systems, where all the empty files will be treated as the same file.


http://reviews.llvm.org/D10423







More information about the cfe-commits mailing list