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

[Richard Smith]

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.


> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150616/861940de/attachment.html>