[PATCH] Do not build modules with missing submodule headers

[Richard Smith]

On Fri, Apr 18, 2014 at 11:37 AM, Ben Langmuir <blangmuir at apple.com> wrote:

>
> On Apr 16, 2014, at 10:33 AM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> I think this would benefit from further refinement.
>
> 1) If a submodule's header is missing, and that submodule would have been
> unavailable anyway (because, say, it requires some feature that's not
> present for the current build), we shouldn't make the parent module
> unavailable.
>
>
> Right, makes sense.
>
>
> 2) If we mark a module unavailable, we should mark all of its submodules
> unavailable too. (The existing approach also had this problem, but it's
> rare for a module to contain both headers and submodules, so I guess it
> seldom happens in practice.)
>
>
> Yep.
>
> Updated patch attached.
>

Thanks! LGTM


> Ben
>
>
>
>
>
> On Wed, Apr 16, 2014 at 5:38 AM, Daniel Jasper <djasper at google.com> wrote:
>
>> I think this makes sense, but I'd also like Richard to take a look.
>>
>>
>> On Tue, Apr 15, 2014 at 6:18 PM, Ben Langmuir <blangmuir at apple.com>wrote:
>>
>>> Hi Daniel,
>>>
>>> Back in r197485 you made it so that missing module headers are marked
>>> unavailable and will fail at build/import time rather than when parsing the
>>> module map file.  This patch fixes the case where a submodule is missing a
>>> header - right now the top-level module will build without the header,
>>> which is awful, because 1) you don’t get a diagnostic for the missing
>>> header and may just get missing symbols, and 2) even after you replace the
>>> missing header the module won’t rebuild because the pcm file doesn’t depend
>>> on that header if it wasn’t included.
>>>
>>> I’m not sure if I did this the right way, since it seems like the
>>> MissingHeader should be on the submodule, but I wasn’t sure if we wanted to
>>> search all of a module’s children in order to figure out what happened.
>>>
>>> Ben
>>>
>>>
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140418/eb8fd386/attachment.html>