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 ? 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?). -- 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
