[cfe-dev] [Modules] Silent textual inclusion when files not found.

[Richard Smith]

On Wed, Jun 3, 2015 at 6:24 PM, Sean Silva <chisophugis at gmail.com> wrote:

> On Wed, Jun 3, 2015 at 8:40 AM, Ben Langmuir <blangmuir at apple.com> wrote:
>
>>
>> On Jun 2, 2015, at 7:57 PM, Sean Silva <chisophugis at gmail.com> wrote:
>>
>> Hi all,
>>
>> Currently we do not seem to issue any form of diagnostic when there are
>> missing headers named in a module map. See the attached test case. Even
>> worse, we will just treat all headers in the module as textual. Hilarity
>> ensues.
>>
>>
>> Yep, this is llvm.org/PR20507
>>
>>
>> Some prior art in this area:
>> Daniel - r197485
>> Ben - r206664
>>
>> Based on these commits, it ostensibly seems that clang does *some* sort
>> of checking for missing files in a module map, but I can't seem to coax
>> clang into doing this in C++ language mode.
>>
>> Looking at the source code, it seems like we end up conflating
>> "unavailable" due to a failed `requires` with "a header is missing". It
>> sounds like we essentially need two notions "unsatisfied `requires`" and
>> "necessary header is missing" (a header guarded by an unsatisfied
>> `requires` does not count as "necessary"). Haven't dug in deep yet, but my
>> hypothesis is that somewhere along the way we silently treat a module with
>> missing headers as though it had an unsatisfied `requires`, leading to us
>> silently neglecting that it was ever in a module at all.
>>
>>
>> Yes, they both come out as the module being “unvavailable”, which is why
>> we don’t import it.  I think the right answer is that attempting to import
>> any unavailable module (even via an auto-import) should be an error.  That
>> is:
>>
>> module MissingHeader {
>>     header “exists.h"
>>     header “doesnt_exist.h”
>> }
>>
>> #include <exists.h> // error, this would import MissingHeader, which is
>> unavailable because we're missing header “doesnt_exist.h"
>>
>> module Top {
>>     header “Top.h”
>>     module A {
>>         requires non_existent
>>        header “A.h”
>>     }
>>     module B {
>>         header “B.h”
>>     }
>> }
>>
>> #include <Top.h> // OK
>> #include <A.h> // error, this would import Top.A, which is unavailable
>> because of the missing requirment
>> #include <B.h> // OK
>>
>> But handling the first case is more important I think.
>>
>>
>> I'm glad to put some effort into fixing this; I spent a good part of
>> today with a bizarre error that I traced back to this and I don't want my
>> users to have to deal with the same. Any pointers would be appreciated.
>>
>>
>> That would be great! I was coincidentally hoping to fix this myself in
>> the next few weeks, but if you beat me to it even better.
>>
>>
> I'm still acclimating myself to the modules code, do you have any concrete
> thoughts on how to do this? I was just planning on looking through for
> everywhere we use "unavailable" happens and figure out a pattern, but if
> you already have an idea, it might save me some time.
>

Something like:
 * make ModuleMap::findModuleForHeader able to produce an unavailable
module, but treat it as being worse than any available module
 * make Preprocessor::HandleIncludeDirective issue an error if its call to
LookupFile produces a SuggestedModule that is unavailable

This should probably happen even if modules is disabled (that is, when we
are parsing module maps but not actually building / using precompiled
module files).

Also, any preferred bikeshed for the names for "unvailable because of
> unsatisfied requires" and "necessary header is missing" concepts in the
> source?
>

If the response to both is the same (as Ben suggested, and I agree) then
I'm not sure that we need to distinguish them anywhere other than the place
where we produce the diagnostic.


> -- Sean Silva
>
>
>>
>> -- Sean Silva
>> <testmoduledepbuildfail.tar>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150604/20764039/attachment.html>