<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 16, 2015 at 7:21 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Tue, Jun 16, 2015 at 7:10 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Tue, Jun 16, 2015 at 2:38 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Fri, Jun 12, 2015 at 8:29 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span>================<br>
Comment at: lib/Lex/ModuleMap.cpp:346-347<br>
@@ -345,4 +345,1 @@<br>
- // Cannot use a module if it is unavailable.<br>
- if (!H.getModule()->isAvailable())<br>
- continue;<br>
if (!Result || isBetterKnownHeader(H, Result))<br>
----------------<br>
</span><span>rsmith wrote:<br>
> 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.)<br>
</span>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?<br>
<br>
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).</blockquote><div><br></div></span><div>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...</div><div><br></div><div>... 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.</div></div></div></div></blockquote><div><br></div></span><div>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?</div></div></div></div></blockquote><div><br></div></span><div>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.</div></div></div></div></blockquote><div><br></div><div>So concretely for the present patch just the change to isBetterKnownHeader to take into account of isAvailable?</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">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.</blockquote><div><br></div></span><div>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.</div></div></div></div></blockquote><div><br></div></span><div>Can you give a more concrete example? I'm still having a hard time imagining this use case.</div></div></div></div></blockquote><div><br></div></span><div>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. </div></div></div></div>
</blockquote></div><br></div><div class="gmail_extra">Okay, I can see that.</div><div class="gmail_extra"><br></div><div class="gmail_extra">-- Sean Silva</div></div>