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

[Richard Smith]

On Fri, Jul 10, 2015 at 1:51 PM, Sean Silva <chisophugis at gmail.com> wrote:

>
>
> On Wed, Jul 8, 2015 at 7:16 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> 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).
>>
>
> What are the semantics of the "current" module and the "implementation of"
> module? I.e. what do they mean in general?
>

They mean that the current file is notionally part of the specified module.
One consequence is that the headers for that module are textually included
rather than being imported. (And for some build systems, only a minimal set
of files necessary for the build are available at compile time, so we
cannot assume that the current module will always be available.)

As for why we have two of these, that seems to simply be a mistake. We
should instead have a single "current module" value and a flag to say
whether or not we're compiling the module interface.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150713/c3671207/attachment.html>