On Fri, Oct 16, 2015 at 6:41 PM, Richard Smith <rich...@metafoo.co.uk> 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. > Yeah, I was wondering whether you were diving into doing that :) Any way you could import this info from the bazel rules you guys already have? -- 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
