<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jun 3, 2015 at 6:24 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Wed, Jun 3, 2015 at 8:40 AM, Ben Langmuir <span dir="ltr"><<a href="mailto:blangmuir@apple.com" target="_blank">blangmuir@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On Jun 2, 2015, at 7:57 PM, Sean Silva <<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>> wrote:</div><br><div><div dir="ltr">Hi all,<div><br></div><div>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.</div></div></div></blockquote><div><br></div></span><div>Yep, this is <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_PR20507&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=CnzuN65ENJ1H9py9XLiRvC_UQz6u3oG6GUNn7_wosSM&m=idlGdt-iQBBOyAfZEHYpM3vnkFXFkclw_N7gtRsKewQ&s=B2W5tYOzj8jWAHd1hWX7DJC5WkSR0Zt7qKAXZMpIRAo&e=" target="_blank">llvm.org/PR20507</a></div><span><div><br></div><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>Some prior art in this area:<div>Daniel - r197485</div><div>Ben - r206664</div></div><div><br></div><div>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.</div><div><br></div><div>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.</div></div></div></blockquote><div><br></div></span><div>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:</div><div><br></div><div><div>module MissingHeader {</div><div> header “exists.h"</div><div> header “doesnt_exist.h”</div><div>}</div><div><br></div><div>#include <exists.h> // error, this would import MissingHeader, which is unavailable because we're missing header “doesnt_exist.h"</div></div><div><br></div><div>module Top {</div><div> header “Top.h”</div><div> module A {</div><div> requires non_existent</div><div> header “A.h”</div><div> }</div><div> module B {</div><div> header “B.h”</div><div> }</div><div>}</div><div><br></div><div>#include <Top.h> // OK</div><div>#include <A.h> // error, this would import Top.A, which is unavailable because of the missing requirment</div><div>#include <B.h> // OK</div><div><br></div><div>But handling the first case is more important I think.</div><span><br><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>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.</div></div></div></blockquote><div><br></div></span><div>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.</div><br></div></div></blockquote><div><br></div></div></div><div>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.</div></div></div></div></blockquote><div><br></div><div>Something like:</div><div> * make ModuleMap::findModuleForHeader able to produce an unavailable module, but treat it as being worse than any available module</div><div> * make Preprocessor::HandleIncludeDirective issue an error if its call to LookupFile produces a SuggestedModule that is unavailable</div><div><br></div><div>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).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Also, any preferred bikeshed for the names for "unvailable because of unsatisfied requires" and "necessary header is missing" concepts in the source?</div></div></div></div></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>-- Sean Silva</div></div>
<span><testmoduledepbuildfail.tar></span></div></blockquote></div><br></div></blockquote></div><br></div></div>
</blockquote></div><br></div></div>