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

[David Blaikie via cfe-commits]

On Fri, May 18, 2018 at 2:22 PM Eric Liu <ioeric at google.com> wrote:

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

Sounds OK - so long as both header and implementation go together is all.

(by the name "Inclusions" I was worried maybe just the headers had been
pulled out, but not their implementation)


>
>> 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/87ee099d/attachment-0001.html>