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