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

Richard Smith richard at metafoo.co.uk
Wed Jul 8 19:16:42 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:
> 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.
Hmm, that's not quite right (`ShouldEnter` will be `false` the second time we reach a header with an include guard too). I think the right thing is: don't diagnose this if the suggested module's top-level module is either the current module or the "implementation of" module (that is, if it's a module we'll /always/ enter textually).


http://reviews.llvm.org/D10423







More information about the cfe-commits mailing list