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