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

[Eric Liu via cfe-commits]

Thanks a lot for looking into this Bruno! The fix looks promising; I'll
give it a try next week :D

On Fri, May 18, 2018 at 10:33 PM David Blaikie via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> 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).
>
David, I'm not exactly sure if I understand... This change pulls both
declarations and implementations into a self-contained library which could
be shared by clang-format and other tools that manipulate #includes. This
seems to be a normal refactoring, and I hope this doesn't break modular
code generation.

>
> 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
>>
> _______________________________________________
> 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/b2559ec0/attachment.html>