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

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 16 18:26:56 PDT 2015


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.


> -- 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/1db0b2f9/attachment.html>


More information about the cfe-commits mailing list