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

Richard Smith richard at metafoo.co.uk
Wed Jul 8 19:13:38 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;
----------------
rsmith wrote:
> 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.
One other thing: we should only diagnose when `ShouldEnter` is `false`. If we're entering the file anyway, we don't need the module to be available; only those files within it that we're actually entering need to be present.


http://reviews.llvm.org/D10423







More information about the cfe-commits mailing list