On Tue, Oct 20, 2015 at 1:38 AM, Manuel Klimek <kli...@google.com> wrote:
> 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 ;) > Thanks, my CMake-fu is weak. Any help from doing it yourself to pointing me in the right direction is much appreciated. > > >> Richard, are there any blockers to exposing a driver flag for explicit >> modules? >> > > Which flag are you missing? > IIRC -emit-module cannot be accessed from the driver? -- Sean Silva > > >> >> -- 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
