<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 20, 2015 at 1:38 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.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_quote"><div><div class="h5"><div dir="ltr">On Tue, Oct 20, 2015 at 5:52 AM Sean Silva <<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>> wrote:<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">On Mon, Oct 19, 2015 at 2:10 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.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_quote"><span><div dir="ltr">On Sat, Oct 17, 2015 at 3:41 AM 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: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">On Fri, Oct 16, 2015 at 6:30 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 Fri, Oct 16, 2015 at 6:26 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, Oct 16, 2015 at 6:25 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 Fri, Oct 16, 2015 at 6:12 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, Oct 16, 2015 at 4:43 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 Fri, Oct 16, 2015 at 4:20 PM, Richard Smith 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: rsmith<br>
Date: Fri Oct 16 18:20:19 2015<br>
New Revision: 250577<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=250577&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=250577&view=rev</a><br>
Log:<br>
[modules] Allow the error when explicitly loading an incompatible module file<br>
via -fmodule-file= to be turned off; in that case, just include the relevant<br>
files textually. This allows module files to be unconditionally passed to all<br>
compile actions via CXXFLAGS, and to be ignored for rules that specify custom<br>
incompatible flags.<br></blockquote><div><br></div></span><div>What direction are you trying to go with this? Are you thinking something like having CMake build a bunch of modules up front?</div></div></div></div></blockquote><div><br></div></span><div>That's certainly one thing you can do with this. Another is that you can make cmake automatically and explicitly build a module for each library, and then provide that for all the dependencies of that library,</div></div></div></div></blockquote><div><br></div></span><div>How does CMake know which headers are part of which library? Strategically named top-level modules in the module map?</div></div></div></div></blockquote><div><br></div></span><div>The idea would be for CMake to generate the module map itself based on the build rules.</div></div></div></div></blockquote><div><br></div></span><div>How would it know which headers to include? Do our ADDITIONAL_HEADER_DIRS things in our CMakeLists.txt have enough information for this?</div></div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Some additional information may need to be added to the CMakeLists to enable this. Some build systems already model the headers for a library, and so already have the requisite information.</div></div></div></div></blockquote><div><br></div></span><div>CMake supports specifying headers for libraries (mainly used for MS VS). If we need this for modules, we'll probably need to update our build rules (which will probably make sense anyway, for a better experience for VS users ;)</div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Nice.</div><div><br></div><div>Brad, do you have any idea how hard it would be to get cmake to generate clang module map files and add explicit module build steps? Basically, the requirements (off the top of my head) are:</div><div>- for each library, generate a module map which is essentially just a list of the headers in that library (it's not just a flat list, but that's the gist of it).</div><div>- for each module map, add a build step that invokes clang on it to say "build the module corresponding to this module map" (it's basically `clang++ path/to/foo.modulemap -o foo.pcm` with a little bit of fluff around it). There is also a dependency from foo.pcm on each of the libraries that library "foo" depends on.</div><div>- for each library $Dep that library $Lib depends on, add $Dep's .pcm file as a dependency of the .o build steps for $Lib. $Dep's .pcm file also needs to be passed on the command line of the .o build steps for $Lib.</div><div><br></div><div>It seems like similar requirements are going to be common in the standardized modules feature (except for the module map I think? Richard?). Basically, in order to avoid redundantly parsing textual headers, you need to run a build step on headers that turns them into some form that can be processed more efficiently than just parsing it. E.g. the build step on slide 36 of this cppcon presentation about the Microsoft experimental modules implementation <a href="https://www.youtube.com/watch?v=RwdQA0pGWa4" target="_blank">https://www.youtube.com/watch?v=RwdQA0pGWa4</a> (slides: <a href="https://goo.gl/t4Eg89" target="_blank">https://goo.gl/t4Eg89</a> ).</div><div><br></div><div>Let me know if there is anything I can do to help (up to and including patches, but I'll need pointers and possibly some hand-holding as I'm unfamiliar with the CMake language and CMake codebase).</div><div><br></div><div>There's also some issues of detecting if the host clang is new enough that we want to use its modules feature and also the issue of detecting modularized system headers if available, but we can hammer those things out as we run into them.</div><div><br></div><div>Manuel, I heard through the grape vine that you were the one that implemented the explicit modules stuff for bazel? Did I miss anything in my list above?</div></div></div></div></blockquote><div><br></div></div></div><div>I think that's about right. We also embed the module maps into the modules, but most of these things only matter for distributed builds at scale.</div><div><br></div><div>Also, I have some experience hacking on cmake, and from my experience I think this shouldn't be too hard to get working (mainly work ;)</div></div></div></blockquote><div><br></div><div>Thanks, my CMake-fu is weak. Any help from doing it yourself to pointing me in the right direction is much appreciated.</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"><span class=""><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>Richard, are there any blockers to exposing a driver flag for explicit modules?</div></div></div></div></blockquote><div><br></div></span><div>Which flag are you missing?</div></div></div></blockquote><div><br></div><div>IIRC -emit-module cannot be accessed from the driver?</div><div><br></div><div>-- Sean Silva</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_quote"><div><div class="h5"><div><br></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><br></div><div>-- Sean Silva</div><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_quote"><div><div><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"><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><font color="#888888"><div>-- Sean Silva</div></font></span><div><div><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"><div><div><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><font color="#888888"><div>-- Sean Silva</div></font></span><div><div><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"><div> with an (error-by-default) warning in the case where the downstream library specifies incompatible compilation flags. You can use this warning flag to turn off the error so you can make progress before you get around to fixing all the incompatible flags.</div><span><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"><div>If that's the case, it would be nice to explain what caused the mismatch, so that the user can look into rectifying it. Otherwise this warning is not directly actionable. The existing diagnostics seemed alright. Demoting them to "error: {{.*}} configuration mismatch" seems like a regression.</div></div></div></div></blockquote><div><br></div></span><div>I agree, it is a regression, and fixing it is high on my list of priorities (sorry for not mentioning that in the commit message).</div><div><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><font color="#888888"><div>-- Sean Silva</div></font></span><div><div><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">
<br>
Modified:<br>
    cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td<br>
    cfe/trunk/lib/Frontend/CompilerInstance.cpp<br>
    cfe/trunk/test/Modules/merge-target-features.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=250577&r1=250576&r2=250577&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=250577&r1=250576&r2=250577&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Fri Oct 16 18:20:19 2015<br>
@@ -172,6 +172,9 @@ def warn_incompatible_analyzer_plugin_ap<br>
 def note_incompatible_analyzer_plugin_api : Note<<br>
     "current API version is '%0', but plugin was compiled with version '%1'">;<br>
<br>
+def warn_module_config_mismatch : Warning<<br>
+  "module file %0 cannot be loaded due to a configuration mismatch with the current "<br>
+  "compilation">, InGroup<DiagGroup<"module-file-config-mismatch">>, DefaultError;<br>
 def err_module_map_not_found : Error<"module map file '%0' not found">,<br>
   DefaultFatal;<br>
 def err_missing_module_name : Error<<br>
<br>
Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=250577&r1=250576&r2=250577&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=250577&r1=250576&r2=250577&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri Oct 16 18:20:19 2015<br>
@@ -1335,15 +1335,24 @@ bool CompilerInstance::loadModuleFile(St<br>
                                                    std::move(Listener));<br>
<br>
   // Try to load the module file.<br>
-  if (ModuleManager->ReadAST(FileName, serialization::MK_ExplicitModule,<br>
-                             SourceLocation(), ASTReader::ARR_None)<br>
-          != ASTReader::Success)<br>
-    return false;<br>
-<br>
+  switch (ModuleManager->ReadAST(FileName, serialization::MK_ExplicitModule,<br>
+                                 SourceLocation(),<br>
+                                 ASTReader::ARR_ConfigurationMismatch)) {<br>
+  case ASTReader::Success:<br>
   // We successfully loaded the module file; remember the set of provided<br>
   // modules so that we don't try to load implicit modules for them.<br>
   ListenerRef.registerAll();<br>
   return true;<br>
+<br>
+  case ASTReader::ConfigurationMismatch:<br>
+    // Ignore unusable module files.<br>
+    getDiagnostics().Report(SourceLocation(), diag::warn_module_config_mismatch)<br>
+        << FileName;<br>
+    return true;<br>
+<br>
+  default:<br>
+    return false;<br>
+  }<br>
 }<br>
<br>
 ModuleLoadResult<br>
<br>
Modified: cfe/trunk/test/Modules/merge-target-features.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-target-features.cpp?rev=250577&r1=250576&r2=250577&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-target-features.cpp?rev=250577&r1=250576&r2=250577&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/merge-target-features.cpp (original)<br>
+++ cfe/trunk/test/Modules/merge-target-features.cpp Fri Oct 16 18:20:19 2015<br>
@@ -20,7 +20,7 @@<br>
 // RUN:   -target-cpu i386 \<br>
 // RUN:   -fsyntax-only merge-target-features.cpp 2>&1 \<br>
 // RUN:   | FileCheck --check-prefix=SUBSET %s<br>
-// SUBSET: AST file was compiled with the target feature'+sse2' but the current translation unit is not<br>
+// SUBSET: error: {{.*}} configuration mismatch<br>
 //<br>
 // RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \<br>
 // RUN:   -iquote Inputs/merge-target-features \<br>
@@ -56,8 +56,7 @@<br>
 // RUN:   -target-cpu i386 -target-feature +cx16 \<br>
 // RUN:   -fsyntax-only merge-target-features.cpp 2>&1 \<br>
 // RUN:   | FileCheck --check-prefix=MISMATCH %s<br>
-// MISMATCH: AST file was compiled with the target feature'+sse2' but the current translation unit is not<br>
-// MISMATCH: current translation unit was compiled with the target feature'+cx16' but the AST file was not<br>
+// MISMATCH: error: {{.*}} configuration mismatch<br>
<br>
 #include "foo.h"<br>
<br>
<br>
<br>
_______________________________________________<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/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div>
_______________________________________________<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/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div></div>
</blockquote></div></div></div></blockquote></div></div></div></div>
</blockquote></div><br></div></div>