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

[Richard Smith]

On Tue, Jun 16, 2015 at 7:10 PM, Sean Silva <chisophugis at gmail.com> wrote:

> On Tue, Jun 16, 2015 at 2:38 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Fri, Jun 12, 2015 at 8:29 PM, Sean Silva <chisophugis at gmail.com>
>> wrote:
>>
>>> ================
>>> Comment at: lib/Lex/ModuleMap.cpp:346-347
>>> @@ -345,4 +345,1 @@
>>> -      // Cannot use a module if it is unavailable.
>>> -      if (!H.getModule()->isAvailable())
>>> -        continue;
>>>        if (!Result || isBetterKnownHeader(H, Result))
>>> ----------------
>>> rsmith wrote:
>>> > Should we also teach `isBetterKnownHeader` to consider an available
>>> module to be better than an unavailable one? (If I have two modules
>>> covering a header file, and one of them requires "blah" and the other
>>> requires "!blah", we should presumably pick the available module.)
>>> I'm having a hard time thinking of a legitimate use for that, or in
>>> general to ever have two modules referencing the same file. Do you have
>>> something in particular in mind?
>>>
>>> But that does raise the larger question is how to make this insensitive
>>> to whatever order we are iterating here (which I assume is determined in
>>> some relatively fragile way by the order of command line arguments or order
>>> in the module map file).
>>
>>
>> That's a good question. We used to try to prefer the current module over
>> all others, and to prefer modules the current module uses over others, but
>> either that became broken at some point or never worked, and you removed
>> the last vestige of it in r239453. We should bring that back and make it
>> work...
>>
>> ... but it's an incomplete solution. One of the options for the remaining
>> cases would be to import *all* the tied-for-best modules for a particular
>> header if it's ambiguous, perhaps with a warning.
>>
>
> That sounds reasonable. Do you still want me to put the band-aid in
> isBetterKnownHeader? Or do you see that as another rule to use to try to
> have things "Just Work (TM)" for users?
>

I'd like the "prefer the current module to anything else; prefer things we
can use over things we can't" rules added. I don't think they're a band-aid
really; if a library provides two interfaces with different sets of
headers, and another module explicitly depends on one of those, we
shouldn't warn about ambiguities with the other one, or diagnose undeclared
uses of the other one.


> From a user's perspective, it is extremely jarring to have the compiler's
>>> behavior depend in a fragile way on such things (like the unix linker
>>> order-dependence fiasco). I would prefer to just have a hard error if two
>>> modules reference the same header, thus we wouldn't even be iterating here
>>> at all, and users will always be immediately alerted to errors.
>>
>>
>> This is a feature that we explicitly support, and some users (ourselves
>> included) rely on. Sometimes you have two logical libraries provided by the
>> same source code that have an overlapping set of headers. It would be more
>> philosophically clean to require such shared headers to be factored out
>> into a separate sub-library that the overlapping libraries can both depend
>> on, but an explicit goal of Clang's module support is to pragmatically
>> support existing code and configurations.
>>
>
> Can you give a more concrete example? I'm still having a hard time
> imagining this use case.
>

Suppose you have a library that has a "light" version and a "full-featured"
version (where most, but perhaps not all, of the "light" headers are part
of the "full-featured" library), and you choose to represent that as two
modules (with overlapping headers) rather than three.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150616/653f4c8d/attachment.html>