r250577 - [modules] Allow the error when explicitly loading an incompatible module file

Fri Oct 16 18:41:29 PDT 2015

On Fri, Oct 16, 2015 at 6:30 PM, Sean Silva <chisophugis at gmail.com> wrote:

> On Fri, Oct 16, 2015 at 6:26 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Fri, Oct 16, 2015 at 6:25 PM, Sean Silva <chisophugis at gmail.com>
>> wrote:
>>
>>> On Fri, Oct 16, 2015 at 6:12 PM, Richard Smith <richard at metafoo.co.uk>
>>> wrote:
>>>
>>>> On Fri, Oct 16, 2015 at 4:43 PM, Sean Silva <chisophugis at gmail.com>
>>>> wrote:
>>>>
>>>>> On Fri, Oct 16, 2015 at 4:20 PM, Richard Smith via cfe-commits <
>>>>> cfe-commits at 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 at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151016/3790c874/attachment.html>