On Thu, Nov 5, 2015 at 1:12 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> On Wed, Nov 4, 2015 at 7:14 PM, Sean Silva via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> On Wed, Nov 4, 2015 at 1:07 PM, Richard Smith <rich...@metafoo.co.uk> >> wrote: >> >>> On Sun, Nov 1, 2015 at 10:20 AM, Manuel Klimek <kli...@google.com> >>> wrote: >>> >>>> On Fri, Oct 23, 2015 at 9:31 PM Sean Silva <chisophu...@gmail.com> >>>> wrote: >>>> >>>>> On Tue, Oct 20, 2015 at 1:52 AM, Manuel Klimek <kli...@google.com> >>>>> wrote: >>>>> >>>>>> 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?) >>>>>> >>>>> >>>>> Actually, despite a decent amount of experimenting and looking at the >>>>> source, I can't seem to find a way to do this without invoking cc1 >>>>> directly. (basically, it seems like there is no way to avoid -emit-obj or >>>>> another not-"-emit-module" action while still getting all the right >>>>> driver-y stuff passed down). Is there some magic invocation? >>>>> >>>> >>>> We use -x c++ -Xclang=emit-module. >>>> >>> >>> We should add a way of doing this without passing cc1 flags through the >>> driver. Maybe: >>> >>> clang -x c++-module my_module_name >>> >>> (...though it's a bit weird to pass something other than a file name as >>> an input to the driver.) >>> >> >> Wouldn't this have to be: clang -x c++-module my_module.modulemap >> -fmodule-name=my_module_name ? >> > > We can pick whatever command-line syntax we like =) > > It makes more sense to me to pass the module map file via > -fmodule-map-file= rather than treating one of the input module maps as a > special case. > > Also, we would need an equivalent of `-fmodule-map-file-home-is-cwd` which >> is necessary for sane explicit module builds. (for the record, I find that >> option name very confusing because despite all my efforts to the contrary, >> I still read it as having a subphrase "home is cwd" which makes it sound >> like it has something to do with `~` expansion or something; maybe we could >> generalize it to `-fmodule-map-header-path-root=.` or something?). >> > > We may also want to be able to specify a different value for each > explicitly-specified module map file. I was thinking about something like: > > -fmodule-map-file=.:foo/bar.modulemap > > I like that! -- Sean Silva > where the piece before the : is the root directory for modules defined in > that module map file. But separate flags could also work if we preserve > their order and use the most recent one prior to the -fmodule-map-file= > argument or similar. > > >> -- Sean Silva >> >> >>> >>> What exactly is not working? >>>> >>>> >>>>> I looked inside of Bazel (current master 76fa4a4) and all -Xclang >>>>> flags seem to be PGO-related. In 2fd9960f you seem to have removed mention >>>>> of them with commit message "Get rid of legacy default features that are >>>>> not needed any more.". Do you guys have private driver patches for >>>>> building >>>>> with modules? >>>>> >>>> >>>> No >>>> >>>> >>>>> Could you push those upstream? Also grepping for -fmodules only finds >>>>> objc related stuff, so I'm wondering how Bazel is even turning on the >>>>> modules feature. >>>>> >>>> >>>> We're actively working on removing all compiler flags from bazel. >>>> Instead, bazel uses a toolchain definition file with all flags. >>>> Unfortunately, the current one does not include support for modules yet, as >>>> we're first working on getting it set up internally. >>>> >>>> >>>>> >>>>> -- Sean Silva >>>>> >>>>> >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> -- 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 >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
