On Tue, Oct 20, 2015 at 10:41 AM Sean Silva <chisophu...@gmail.com> wrote:
> 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? > Ah, you're right (well, all flags can be accessed via the driver by saying -Xclang -flag, right?) > > -- 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
