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

[Eric Liu via cfe-commits]

Sorry about the bad naming... I hope I could come up with something less
confusing. Thanks for the clarification!

On Fri, May 18, 2018 at 11:25 PM David Blaikie <dblaikie at gmail.com> wrote:

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