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

Sean Silva via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 19 20:21:13 PDT 2015 

On Fri, Oct 16, 2015 at 6:41 PM, Richard Smith <richard at metafoo.co.uk>
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.
>

Yeah, I was wondering whether you were diving into doing that :) Any way
you could import this info from the bazel rules you guys already have?

-- Sean Silva


>
>
>> -- 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/20151019/75795ec0/attachment-0001.html>

More information about the cfe-commits mailing list