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

Sean Silva via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 16 18:30:56 PDT 2015


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?

-- 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/d52349ce/attachment-0001.html>


More information about the cfe-commits mailing list