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 <dblai...@gmail.com> wrote:
>
>
> On Fri, May 18, 2018 at 2:22 PM Eric Liu <ioe...@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@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@lists.llvm.org> wrote:
>>>
>>>>
>>>>
>>>> On Fri, May 18, 2018 at 12:46 PM Bruno Cardoso Lopes <
>>>> bruno.card...@gmail.com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Fri, May 18, 2018 at 11:54 AM Vedant Kumar via cfe-commits <
>>>>> cfe-commits@lists.llvm.org> wrote:
>>>>>
>>>>>> On May 18, 2018, at 11:48 AM, Eric Liu <ioe...@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@lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
