On Tue, Oct 20, 2015 at 5:52 AM Sean Silva <chisophu...@gmail.com> wrote:
> On Mon, Oct 19, 2015 at 2:10 AM, Manuel Klimek <kli...@google.com> wrote: > >> On Sat, Oct 17, 2015 at 3:41 AM Richard Smith via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> On Fri, Oct 16, 2015 at 6:30 PM, Sean Silva <chisophu...@gmail.com> >>> wrote: >>> >>>> On Fri, Oct 16, 2015 at 6:26 PM, Richard Smith <rich...@metafoo.co.uk> >>>> wrote: >>>> >>>>> On Fri, Oct 16, 2015 at 6:25 PM, Sean Silva <chisophu...@gmail.com> >>>>> wrote: >>>>> >>>>>> On Fri, Oct 16, 2015 at 6:12 PM, Richard Smith <rich...@metafoo.co.uk >>>>>> > wrote: >>>>>> >>>>>>> On Fri, Oct 16, 2015 at 4:43 PM, Sean Silva <chisophu...@gmail.com> >>>>>>> wrote: >>>>>>> >>>>>>>> On Fri, Oct 16, 2015 at 4:20 PM, Richard Smith via cfe-commits < >>>>>>>> cfe-commits@lists.llvm.org> wrote: >>>>>>>> >>>>>>>>> Author: rsmith >>>>>>>>> Date: Fri Oct 16 18:20:19 2015 >>>>>>>>> New Revision: 250577 >>>>>>>>> >>>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=250577&view=rev >>>>>>>>> Log: >>>>>>>>> [modules] Allow the error when explicitly loading an incompatible >>>>>>>>> module file >>>>>>>>> via -fmodule-file= to be turned off; in that case, just include >>>>>>>>> the relevant >>>>>>>>> files textually. This allows module files to be unconditionally >>>>>>>>> passed to all >>>>>>>>> compile actions via CXXFLAGS, and to be ignored for rules that >>>>>>>>> specify custom >>>>>>>>> incompatible flags. >>>>>>>>> >>>>>>>> >>>>>>>> What direction are you trying to go with this? Are you thinking >>>>>>>> something like having CMake build a bunch of modules up front? >>>>>>>> >>>>>>> >>>>>>> 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, >>>>>>> >>>>>> >>>>>> How does CMake know which headers are part of which library? >>>>>> Strategically named top-level modules in the module map? >>>>>> >>>>> >>>>> The idea would be for CMake to generate the module map itself based on >>>>> the build rules. >>>>> >>>> >>>> How would it know which headers to include? Do >>>> our ADDITIONAL_HEADER_DIRS things in our CMakeLists.txt have enough >>>> information for this? >>>> >>> >>> 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. >>> >> >> 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 ;) >> > > Nice. > > 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: > - 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). > - 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. > - 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. > > 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 https://www.youtube.com/watch?v=RwdQA0pGWa4 > (slides: https://goo.gl/t4Eg89 ). > > 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). > > 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. > > 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? > 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. 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 ;) > Richard, are there any blockers to exposing a driver flag for explicit > modules? > Which flag are you missing? > > -- Sean Silva > > >> >> >>> >>> >>>> -- Sean Silva >>>> >>>> >>>>> >>>>> >>>>>> -- Sean Silva >>>>>> >>>>>> >>>>>>> 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. >>>>>>> >>>>>>> >>>>>>>> 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. >>>>>>>> >>>>>>> >>>>>>> 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). >>>>>>> >>>>>>> -- Sean Silva >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> Modified: >>>>>>>>> cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td >>>>>>>>> cfe/trunk/lib/Frontend/CompilerInstance.cpp >>>>>>>>> cfe/trunk/test/Modules/merge-target-features.cpp >>>>>>>>> >>>>>>>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td >>>>>>>>> URL: >>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=250577&r1=250576&r2=250577&view=diff >>>>>>>>> >>>>>>>>> ============================================================================== >>>>>>>>> --- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td >>>>>>>>> (original) >>>>>>>>> +++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Fri >>>>>>>>> Oct 16 18:20:19 2015 >>>>>>>>> @@ -172,6 +172,9 @@ def warn_incompatible_analyzer_plugin_ap >>>>>>>>> def note_incompatible_analyzer_plugin_api : Note< >>>>>>>>> "current API version is '%0', but plugin was compiled with >>>>>>>>> version '%1'">; >>>>>>>>> >>>>>>>>> +def warn_module_config_mismatch : Warning< >>>>>>>>> + "module file %0 cannot be loaded due to a configuration >>>>>>>>> mismatch with the current " >>>>>>>>> + "compilation">, >>>>>>>>> InGroup<DiagGroup<"module-file-config-mismatch">>, DefaultError; >>>>>>>>> def err_module_map_not_found : Error<"module map file '%0' not >>>>>>>>> found">, >>>>>>>>> DefaultFatal; >>>>>>>>> def err_missing_module_name : Error< >>>>>>>>> >>>>>>>>> Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp >>>>>>>>> URL: >>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=250577&r1=250576&r2=250577&view=diff >>>>>>>>> >>>>>>>>> ============================================================================== >>>>>>>>> --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) >>>>>>>>> +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri Oct 16 >>>>>>>>> 18:20:19 2015 >>>>>>>>> @@ -1335,15 +1335,24 @@ bool CompilerInstance::loadModuleFile(St >>>>>>>>> >>>>>>>>> std::move(Listener)); >>>>>>>>> >>>>>>>>> // Try to load the module file. >>>>>>>>> - if (ModuleManager->ReadAST(FileName, >>>>>>>>> serialization::MK_ExplicitModule, >>>>>>>>> - SourceLocation(), >>>>>>>>> ASTReader::ARR_None) >>>>>>>>> - != ASTReader::Success) >>>>>>>>> - return false; >>>>>>>>> - >>>>>>>>> + switch (ModuleManager->ReadAST(FileName, >>>>>>>>> serialization::MK_ExplicitModule, >>>>>>>>> + SourceLocation(), >>>>>>>>> + >>>>>>>>> ASTReader::ARR_ConfigurationMismatch)) { >>>>>>>>> + case ASTReader::Success: >>>>>>>>> // We successfully loaded the module file; remember the set of >>>>>>>>> provided >>>>>>>>> // modules so that we don't try to load implicit modules for >>>>>>>>> them. >>>>>>>>> ListenerRef.registerAll(); >>>>>>>>> return true; >>>>>>>>> + >>>>>>>>> + case ASTReader::ConfigurationMismatch: >>>>>>>>> + // Ignore unusable module files. >>>>>>>>> + getDiagnostics().Report(SourceLocation(), >>>>>>>>> diag::warn_module_config_mismatch) >>>>>>>>> + << FileName; >>>>>>>>> + return true; >>>>>>>>> + >>>>>>>>> + default: >>>>>>>>> + return false; >>>>>>>>> + } >>>>>>>>> } >>>>>>>>> >>>>>>>>> ModuleLoadResult >>>>>>>>> >>>>>>>>> Modified: cfe/trunk/test/Modules/merge-target-features.cpp >>>>>>>>> URL: >>>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-target-features.cpp?rev=250577&r1=250576&r2=250577&view=diff >>>>>>>>> >>>>>>>>> ============================================================================== >>>>>>>>> --- cfe/trunk/test/Modules/merge-target-features.cpp (original) >>>>>>>>> +++ cfe/trunk/test/Modules/merge-target-features.cpp Fri Oct 16 >>>>>>>>> 18:20:19 2015 >>>>>>>>> @@ -20,7 +20,7 @@ >>>>>>>>> // RUN: -target-cpu i386 \ >>>>>>>>> // RUN: -fsyntax-only merge-target-features.cpp 2>&1 \ >>>>>>>>> // RUN: | FileCheck --check-prefix=SUBSET %s >>>>>>>>> -// SUBSET: AST file was compiled with the target feature'+sse2' >>>>>>>>> but the current translation unit is not >>>>>>>>> +// SUBSET: error: {{.*}} configuration mismatch >>>>>>>>> // >>>>>>>>> // RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \ >>>>>>>>> // RUN: -iquote Inputs/merge-target-features \ >>>>>>>>> @@ -56,8 +56,7 @@ >>>>>>>>> // RUN: -target-cpu i386 -target-feature +cx16 \ >>>>>>>>> // RUN: -fsyntax-only merge-target-features.cpp 2>&1 \ >>>>>>>>> // RUN: | FileCheck --check-prefix=MISMATCH %s >>>>>>>>> -// MISMATCH: AST file was compiled with the target feature'+sse2' >>>>>>>>> but the current translation unit is not >>>>>>>>> -// MISMATCH: current translation unit was compiled with the >>>>>>>>> target feature'+cx16' but the AST file was not >>>>>>>>> +// MISMATCH: error: {{.*}} configuration mismatch >>>>>>>>> >>>>>>>>> #include "foo.h" >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> cfe-commits mailing list >>>>>>>>> cfe-commits@lists.llvm.org >>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits