<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 5 June 2017 at 09:35, David Blaikie via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</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_quote"><div><div class="m_-6072473267255232954h5"><div dir="ltr">On Mon, May 29, 2017 at 10:23 PM Richard Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rsmith<br>
Date: Tue May 30 00:22:59 2017<br>
New Revision: 304190<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=304190&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=304190&view=rev</a><br>
Log:<br>
Diagnose attempts to build a preprocessed module that defines an unavailable submodule.<br>
<br>
The errors we would otherwise get are incomprehensible, as we would enter the<br>
module but not make its contents visible to itself.<br>
<br>
Added:<br>
    cfe/trunk/test/Modules/preproc<wbr>ess-unavailable.cpp<br>
Modified:<br>
    cfe/trunk/include/clang/Basic/<wbr>DiagnosticLexKinds.td<br>
    cfe/trunk/lib/Lex/Pragma.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/<wbr>DiagnosticLexKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=304190&r1=304189&r2=304190&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/include/clang/<wbr>Basic/DiagnosticLexKinds.td?<wbr>rev=304190&r1=304189&r2=<wbr>304190&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/Basic/<wbr>DiagnosticLexKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/<wbr>DiagnosticLexKinds.td Tue May 30 00:22:59 2017<br>
@@ -525,6 +525,8 @@ def err_pp_module_begin_without_mo<wbr>dule_e<br>
 def err_pp_module_end_without_modu<wbr>le_begin : Error<<br>
   "no matching '#pragma clang module begin' for this "<br>
   "'#pragma clang module end'">;<br>
+def note_pp_module_begin_here : Note<<br>
+  "entering module '%0' due to this pragma">;<br>
<br>
 def err_defined_macro_name : Error<"'defined' cannot be used as a macro name">;<br>
 def err_paste_at_start : Error<<br>
<br>
Modified: cfe/trunk/lib/Lex/Pragma.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Pragma.cpp?rev=304190&r1=304189&r2=304190&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/Lex/Pragma<wbr>.cpp?rev=304190&r1=304189&r2=<wbr>304190&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Lex/Pragma.cpp (original)<br>
+++ cfe/trunk/lib/Lex/Pragma.cpp Tue May 30 00:22:59 2017<br>
@@ -1407,6 +1407,24 @@ struct PragmaModuleBeginHandler : public<br>
       M = NewM;<br>
     }<br>
<br>
+    // If the module isn't available, it doesn't make sense to enter it.<br>
+    if (!M->isAvailable()) {<br>
+      Module::Requirement Requirement;<br>
+      Module::UnresolvedHeaderDirect<wbr>ive MissingHeader;<br>
+      (void)M->isAvailable(PP.getLan<wbr>gOpts(), PP.getTargetInfo(),<br>
+                           Requirement, MissingHeader);<br></blockquote></div></div><div><br>This looks a tad weird ^ should this function have a different name (or a version of the function that only does the side-effecting work that I'm guessing is desired here)?<br></div></div></div></blockquote><div><br></div><div>It's not exactly producing side-effects; rather, it's filling in output parameters (and we happen to have already checked the primary return value as an 'optimization' here). But it seems like this isn't worth the confusion, plus we repeat this code in four (!) different places. Tidied up and factored out the common code in r304728 (along with a minor improvement to diagnostic quality in one of the four repetitions).</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_quote"><div></div><div><div class="m_-6072473267255232954h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+      if (MissingHeader.FileNameLoc.isV<wbr>alid()) {<br>
+        PP.Diag(MissingHeader.FileName<wbr>Loc, diag::err_module_header_missin<wbr>g)<br>
+          << MissingHeader.IsUmbrella << MissingHeader.FileName;<br>
+      } else {<br>
+        PP.Diag(M->DefinitionLoc, diag::err_module_unavailable)<br>
+          << M->getFullModuleName() << Requirement.second << Requirement.first;<br>
+      }<br>
+      PP.Diag(BeginLoc, diag::note_pp_module_begin_her<wbr>e)<br>
+        << M->getTopLevelModuleName();<br>
+      return;<br>
+    }<br>
+<br>
     // Enter the scope of the submodule.<br>
     PP.EnterSubmodule(M, BeginLoc, /*ForPragma*/true);<br>
     PP.EnterAnnotationToken(Sourc<wbr>eRange(BeginLoc, ModuleName.back().second),<br>
<br>
Added: cfe/trunk/test/Modules/preproc<wbr>ess-unavailable.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/preprocess-unavailable.cpp?rev=304190&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/Modules/<wbr>preprocess-unavailable.cpp?<wbr>rev=304190&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Modules/preproc<wbr>ess-unavailable.cpp (added)<br>
+++ cfe/trunk/test/Modules/preproc<wbr>ess-unavailable.cpp Tue May 30 00:22:59 2017<br>
@@ -0,0 +1,12 @@<br>
+// RUN: %clang_cc1 -x c++-module-map %s -fmodule-name=a -verify<br>
+module a {<br>
+  module b {<br>
+    requires cplusplus11<br>
+  }<br>
+}<br>
+#pragma clang module contents<br>
+// expected-error@3 {{module 'a.b' requires feature 'cplusplus11'}}<br>
+#pragma clang module begin a.b // expected-note {{entering module 'a' due to this pragma}}<br>
+int f();<br>
+int g() { f(); }<br>
+#pragma clang module end // expected-error {{no matching '#pragma clang module begin'}}<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div></div>
<br>______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div>