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

Manuel Klimek via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 19 02:10:44 PDT 2015 

On Sat, Oct 17, 2015 at 3:41 AM Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

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

CMake supports specifying headers for libraries (mainly used for MS VS). If
we need this for modules, we'll probably need to update our build rules
(which will probably make sense anyway, for a better experience for VS
users ;)


>
>
>> -- 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
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>> _______________________________________________
> 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/20151019/120c0f03/attachment-0001.html>

More information about the cfe-commits mailing list