r332720 - Move #include manipulation code to new lib/Tooling/Inclusions.

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Fri May 18 13:33:27 PDT 2018


I haven't looked in detail here, but in general: Don't split an
implementation and its headers into different notional libraries, as that
breaks modular code generation (an implementation and its headers usually
have circular dependencies - inline functions that call non-inline
functions, and the other way around & so if they're in two different
libraries they won't be able to link (due to the way unix linkers
work/dependencies are resolved in a single pass, never looping back around).

On Fri, May 18, 2018 at 1:10 PM Bruno Cardoso Lopes via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

>
>
> On Fri, May 18, 2018 at 12:46 PM Bruno Cardoso Lopes <
> bruno.cardoso at gmail.com> wrote:
>
>>
>>
>> On Fri, May 18, 2018 at 11:54 AM Vedant Kumar via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> On May 18, 2018, at 11:48 AM, Eric Liu <ioeric at google.com> wrote:
>>>
>>>
>>> So I have reverted this with r332751.
>>>
>>>
>>> Thanks!
>>>
>>>
>>> I can't see how this introduced cyclic dependencies in module build, as
>>> the dependencies should be clangTooling -> clangFormat ->
>>> clangToolingInclusions. I'm wondering if there is any module configurations
>>> that I need to update to make this work. Right now, module doesn't seem to
>>> make any difference between clangTooling and clangToolingInclusions...
>>> I'd appreciate it if someone who knows how clang module build is set up
>>> could help take a look.
>>>
>>>
>>> + Bruno & David who have more experience in this area than I do.
>>>
>>
>> Gonna try to reproduce and take a look!
>>
>
> I could reproduce it. You should be good to go if you add another top
> level module for Inclusions (and break the dep):
>
> --- a/include/clang/module.modulemap
> +++ b/include/clang/module.modulemap
> @@ -153,3 +153,8 @@ module Clang_ToolingCore {
>    requires cplusplus
>    umbrella "Tooling/Core" module * { export * }
>  }
> +
> +module Clang_ToolingInclusions {
> +  requires cplusplus
> +  umbrella "Tooling/Inclusions" module * { export * }
> +}
>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc
> _______________________________________________
> 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/20180518/1facc9bb/attachment.html>


More information about the cfe-commits mailing list