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