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

2018-05-18 Thread 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  wrote:

>
>
> On Fri, May 18, 2018 at 2:22 PM Eric Liu  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  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

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

2018-05-18 Thread David Blaikie via cfe-commits 
On Fri, May 18, 2018 at 2:22 PM Eric Liu  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  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

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

2018-05-18 Thread 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@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  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

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

2018-05-18 Thread David Blaikie via cfe-commits 
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).

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

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

2018-05-18 Thread Bruno Cardoso Lopes via cfe-commits 
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  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

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

2018-05-18 Thread Bruno Cardoso Lopes via cfe-commits 
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  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!


>
>
> Unfortunately, I couldn't reproduce this on my workstation as the module
> build seemed to be broken due to other errors.
>
>
> The build is dependent on having a set of modularized system headers --
> perhaps that's the issue.
>
>
> I was seeing a lot of warnings; the clang I used to build llvm was built
> near HEAD. Maybe my clang is too new?
>
>
> This sounds a bit suspicious. Can new clang diagnostics land without clean
> stage2 builds? If so, we can look into enabling -Werror on our stage2 bots
> to catch this sort of thing earlier.
>
> vedant
>
>
> Thanks,
> Eric
>
> On Fri, May 18, 2018 at 7:52 PM Eric Liu  wrote:
>
>> Sorry, I'll look into it right now. Will revert it if I can't find a
>> quick fix.
>>
>> On Fri, May 18, 2018, 19:49 Amara Emerson  wrote:
>>
>>> Hi Eric,
>>>
>>> Green dragon buildbots have started failing too, e.g.:
>>> http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/10222/
>>>
>>> If you don’t have a quick fix can you please revert it.
>>>
>>> Thanks,
>>> Amara
>>>
>>>
>>> On May 18, 2018, at 7:46 PM, Eric Liu  wrote:
>>>
>>> Hi Vedant,
>>>
>>> It seems that your build was not using cmake files in the source tree?
>>> lib/Tooling/Inclusions/ is a (new) standalone library
>>> (clangToolingInclusions, similar to clangToolingCore). You might need
>>> update your build to reflect this change. Let me know if you have any
>>> further question.
>>>
>>> Thanks,
>>> Eric
>>>
>>> On Fri, May 18, 2018 at 6:41 PM Vedant Kumar  wrote:
>>>
 Hi Eric,

 I think there might be a cyclic dependency issue here, possibly one
 that's only visible with LLVM_ENABLE_MODULES=On. I see:

 While building module 'Clang_Tooling' imported from /Users/vsk/src/
 llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:

 While building module 'Clang_Format' imported from /Users/vsk/src/
 llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:

 In file included from :1:


 /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Format/Format.h:20:10:
 fatal error: cyclic dependency in module 'Clang_Tooling': Clang_Tooling $>
 Clang_Format -> Clang_Tooling

 #include "clang/Tooling/Inclusions/IncludeStyle.h"
  ^
 While building module 'Clang_Tooling' imported from /Users/vsk/src/
 llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
 In file included from :22:

 In file included from /Users/vsk/src/llvm.org
 -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringAction.h:14:
 In file included from /Users/vsk/src/llvm.org
 -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRules.h:14:
 In file included from /Users/vsk/src/llvm.org
 -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h:16:
 In file included from /Users/vsk/src/llvm.org
 -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:14:
 /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:10:
 fatal error: could not build module 'Clang_Format'
 #include "clang/Format/Format.h"
  ^~~
 /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:10:
 fatal error: could not build module 'Clang_Tooling'
 #include "clang/Tooling/Execution.h"

  ^~~
 3 errors generated.

 Could you take a look?

 thanks,
 vedant

 > On May 18, 2018, at 7:16 AM, Eric Liu via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:
 >
 > Author: ioeric
 > Date: Fri May 18 07:16:37 2018
 > New Revision: 332720
 >
 > URL: http://llvm.org/viewvc/llvm-project?rev=332720=rev
 > Log:
 > Move #include manipulation code to new lib/Tooling/Inclusions.
 >
 > Summary:
 > clangToolingCore is linked into almost everything (incl. clang), but
 > not few tools need #include manipulation at this

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

2018-05-18 Thread Vedant Kumar via cfe-commits 
> On May 18, 2018, at 11:48 AM, Eric Liu  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.


> Unfortunately, I couldn't reproduce this on my workstation as the module 
> build seemed to be broken due to other errors.

The build is dependent on having a set of modularized system headers -- perhaps 
that's the issue.


> I was seeing a lot of warnings; the clang I used to build llvm was built near 
> HEAD. Maybe my clang is too new?

This sounds a bit suspicious. Can new clang diagnostics land without clean 
stage2 builds? If so, we can look into enabling -Werror on our stage2 bots to 
catch this sort of thing earlier.

vedant

> 
> Thanks,
> Eric
> 
> On Fri, May 18, 2018 at 7:52 PM Eric Liu  > wrote:
> Sorry, I'll look into it right now. Will revert it if I can't find a quick 
> fix.
> 
> On Fri, May 18, 2018, 19:49 Amara Emerson  > wrote:
> Hi Eric,
> 
> Green dragon buildbots have started failing too, e.g.: 
> http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/10222/ 
> 
> 
> If you don’t have a quick fix can you please revert it.
> 
> Thanks,
> Amara
> 
> 
>> On May 18, 2018, at 7:46 PM, Eric Liu > > wrote:
>> 
>> Hi Vedant,
>> 
>> It seems that your build was not using cmake files in the source tree? 
>> lib/Tooling/Inclusions/ is a (new) standalone library 
>> (clangToolingInclusions, similar to clangToolingCore). You might need update 
>> your build to reflect this change. Let me know if you have any further 
>> question.
>> 
>> Thanks,
>> Eric
>> 
>> On Fri, May 18, 2018 at 6:41 PM Vedant Kumar > > wrote:
>> Hi Eric,
>> 
>> I think there might be a cyclic dependency issue here, possibly one that's 
>> only visible with LLVM_ENABLE_MODULES=On. I see:
>> 
>> While building module 'Clang_Tooling' imported from /Users/vsk/src/llvm.org 
>> -master/llvm/tools/clang/lib/Tooling/Execution.cpp:10: 
>> 
>> While building module 'Clang_Format' imported from /Users/vsk/src/llvm.org 
>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:
>>
>> In file included from :1:   
>>  
>>
>> /Users/vsk/src/llvm.org 
>> -master/llvm/tools/clang/include/clang/Format/Format.h:20:10:
>>  fatal error: cyclic dependency in module 'Clang_Tooling': Clang_Tooling $> 
>> Clang_Format -> Clang_Tooling
>> 
>> #include "clang/Tooling/Inclusions/IncludeStyle.h"
>>  ^  
>> While building module 'Clang_Tooling' imported from /Users/vsk/src/llvm.org 
>> -master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
>> In file included from :22:  
>> 
>> In file included from /Users/vsk/src/llvm.org 
>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringAction.h:14:
>> In file included from /Users/vsk/src/llvm.org 
>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRules.h:14:
>> In file included from /Users/vsk/src/llvm.org 
>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h:16:
>> In file included from /Users/vsk/src/llvm.org 
>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:14:
>> /Users/vsk/src/llvm.org 
>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:10:
>>  fatal error: could not build module 'Clang_Format'
>> #include "clang/Format/Format.h"
>>  ^~~
>> /Users/vsk/src/llvm.org 
>> -master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:10: 
>> fatal error: could not build module 'Clang_Tooling'
>> #include "clang/Tooling/Execution.h" 
>>
>>  ^~~
>> 3 errors generated.
>> 
>> Could you take a look?
>> 
>> thanks,
>>

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

2018-05-18 Thread Eric Liu via cfe-commits 
So I have reverted this with r332751.

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.

Unfortunately, I couldn't reproduce this on my workstation as the module
build seemed to be broken due to other errors. I was seeing a lot of
warnings; the clang I used to build llvm was built near HEAD. Maybe my
clang is too new?

Thanks,
Eric

On Fri, May 18, 2018 at 7:52 PM Eric Liu  wrote:

> Sorry, I'll look into it right now. Will revert it if I can't find a quick
> fix.
>
> On Fri, May 18, 2018, 19:49 Amara Emerson  wrote:
>
>> Hi Eric,
>>
>> Green dragon buildbots have started failing too, e.g.:
>> http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/10222/
>>
>> If you don’t have a quick fix can you please revert it.
>>
>> Thanks,
>> Amara
>>
>>
>> On May 18, 2018, at 7:46 PM, Eric Liu  wrote:
>>
>> Hi Vedant,
>>
>> It seems that your build was not using cmake files in the source tree? 
>> lib/Tooling/Inclusions/
>> is a (new) standalone library (clangToolingInclusions, similar to
>> clangToolingCore). You might need update your build to reflect this change.
>> Let me know if you have any further question.
>>
>> Thanks,
>> Eric
>>
>> On Fri, May 18, 2018 at 6:41 PM Vedant Kumar  wrote:
>>
>>> Hi Eric,
>>>
>>> I think there might be a cyclic dependency issue here, possibly one
>>> that's only visible with LLVM_ENABLE_MODULES=On. I see:
>>>
>>> While building module 'Clang_Tooling' imported from /Users/vsk/src/
>>> llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
>>>
>>> While building module 'Clang_Format' imported from /Users/vsk/src/
>>> llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:
>>>
>>> In file included from :1:
>>>
>>>
>>> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Format/Format.h:20:10:
>>> fatal error: cyclic dependency in module 'Clang_Tooling': Clang_Tooling $>
>>> Clang_Format -> Clang_Tooling
>>>
>>> #include "clang/Tooling/Inclusions/IncludeStyle.h"
>>>  ^
>>> While building module 'Clang_Tooling' imported from /Users/vsk/src/
>>> llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
>>> In file included from :22:
>>>
>>> In file included from /Users/vsk/src/llvm.org
>>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringAction.h:14:
>>> In file included from /Users/vsk/src/llvm.org
>>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRules.h:14:
>>> In file included from /Users/vsk/src/llvm.org
>>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h:16:
>>> In file included from /Users/vsk/src/llvm.org
>>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:14:
>>> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:10:
>>> fatal error: could not build module 'Clang_Format'
>>> #include "clang/Format/Format.h"
>>>  ^~~
>>> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:10:
>>> fatal error: could not build module 'Clang_Tooling'
>>> #include "clang/Tooling/Execution.h"
>>>
>>>  ^~~
>>> 3 errors generated.
>>>
>>> Could you take a look?
>>>
>>> thanks,
>>> vedant
>>>
>>> > On May 18, 2018, at 7:16 AM, Eric Liu via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>> >
>>> > Author: ioeric
>>> > Date: Fri May 18 07:16:37 2018
>>> > New Revision: 332720
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=332720=rev
>>> > Log:
>>> > Move #include manipulation code to new lib/Tooling/Inclusions.
>>> >
>>> > Summary:
>>> > clangToolingCore is linked into almost everything (incl. clang), but
>>> > not few tools need #include manipulation at this point. So pull this
>>> into a
>>> > separate library in Tooling.
>>> >
>>> > Reviewers: ilya-biryukov
>>> >
>>> > Subscribers: klimek, mgorny, cfe-commits, thakis
>>> >
>>> > Differential Revision: https://reviews.llvm.org/D47068
>>> >
>>> > Added:
>>> >cfe/trunk/include/clang/Tooling/Inclusions/
>>> >cfe/trunk/include/clang/Tooling/Inclusions/HeaderIncludes.h
>>> >  - copied, changed from r332717,
>>> cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
>>> >cfe/trunk/include/clang/Tooling/Inclusions/IncludeStyle.h
>>> >  - copied, changed from r332717,
>>> cfe/trunk/include/clang/Tooling/Core/IncludeStyle.h
>>> >cfe/trunk/lib/Tooling/Inclusions/
>>> >cfe/trunk/lib/Tooling/Inclusions/CMakeLists.txt
>>> >  -

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

2018-05-18 Thread Eric Liu via cfe-commits 
Sorry, I'll look into it right now. Will revert it if I can't find a quick
fix.

On Fri, May 18, 2018, 19:49 Amara Emerson  wrote:

> Hi Eric,
>
> Green dragon buildbots have started failing too, e.g.:
> http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/10222/
>
> If you don’t have a quick fix can you please revert it.
>
> Thanks,
> Amara
>
>
> On May 18, 2018, at 7:46 PM, Eric Liu  wrote:
>
> Hi Vedant,
>
> It seems that your build was not using cmake files in the source tree? 
> lib/Tooling/Inclusions/
> is a (new) standalone library (clangToolingInclusions, similar to
> clangToolingCore). You might need update your build to reflect this change.
> Let me know if you have any further question.
>
> Thanks,
> Eric
>
> On Fri, May 18, 2018 at 6:41 PM Vedant Kumar  wrote:
>
>> Hi Eric,
>>
>> I think there might be a cyclic dependency issue here, possibly one
>> that's only visible with LLVM_ENABLE_MODULES=On. I see:
>>
>> While building module 'Clang_Tooling' imported from /Users/vsk/src/
>> llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
>>
>> While building module 'Clang_Format' imported from /Users/vsk/src/
>> llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:
>>
>> In file included from :1:
>>
>>
>> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Format/Format.h:20:10:
>> fatal error: cyclic dependency in module 'Clang_Tooling': Clang_Tooling $>
>> Clang_Format -> Clang_Tooling
>>
>> #include "clang/Tooling/Inclusions/IncludeStyle.h"
>>  ^
>> While building module 'Clang_Tooling' imported from /Users/vsk/src/
>> llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
>> In file included from :22:
>>
>> In file included from /Users/vsk/src/llvm.org
>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringAction.h:14:
>> In file included from /Users/vsk/src/llvm.org
>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRules.h:14:
>> In file included from /Users/vsk/src/llvm.org
>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h:16:
>> In file included from /Users/vsk/src/llvm.org
>> -master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:14:
>> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:10:
>> fatal error: could not build module 'Clang_Format'
>> #include "clang/Format/Format.h"
>>  ^~~
>> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:10:
>> fatal error: could not build module 'Clang_Tooling'
>> #include "clang/Tooling/Execution.h"
>>
>>  ^~~
>> 3 errors generated.
>>
>> Could you take a look?
>>
>> thanks,
>> vedant
>>
>> > On May 18, 2018, at 7:16 AM, Eric Liu via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>> >
>> > Author: ioeric
>> > Date: Fri May 18 07:16:37 2018
>> > New Revision: 332720
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=332720=rev
>> > Log:
>> > Move #include manipulation code to new lib/Tooling/Inclusions.
>> >
>> > Summary:
>> > clangToolingCore is linked into almost everything (incl. clang), but
>> > not few tools need #include manipulation at this point. So pull this
>> into a
>> > separate library in Tooling.
>> >
>> > Reviewers: ilya-biryukov
>> >
>> > Subscribers: klimek, mgorny, cfe-commits, thakis
>> >
>> > Differential Revision: https://reviews.llvm.org/D47068
>> >
>> > Added:
>> >cfe/trunk/include/clang/Tooling/Inclusions/
>> >cfe/trunk/include/clang/Tooling/Inclusions/HeaderIncludes.h
>> >  - copied, changed from r332717,
>> cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
>> >cfe/trunk/include/clang/Tooling/Inclusions/IncludeStyle.h
>> >  - copied, changed from r332717,
>> cfe/trunk/include/clang/Tooling/Core/IncludeStyle.h
>> >cfe/trunk/lib/Tooling/Inclusions/
>> >cfe/trunk/lib/Tooling/Inclusions/CMakeLists.txt
>> >  - copied, changed from r332717,
>> cfe/trunk/lib/Tooling/Core/CMakeLists.txt
>> >cfe/trunk/lib/Tooling/Inclusions/HeaderIncludes.cpp
>> >  - copied, changed from r332717,
>> cfe/trunk/lib/Tooling/Core/HeaderIncludes.cpp
>> >cfe/trunk/lib/Tooling/Inclusions/IncludeStyle.cpp
>> >  - copied, changed from r332717,
>> cfe/trunk/lib/Tooling/Core/IncludeStyle.cpp
>> > Removed:
>> >cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
>> >cfe/trunk/include/clang/Tooling/Core/IncludeStyle.h
>> >cfe/trunk/lib/Tooling/Core/HeaderIncludes.cpp
>> >cfe/trunk/lib/Tooling/Core/IncludeStyle.cpp
>> > Modified:
>> >cfe/trunk/include/clang/Format/Format.h
>> >cfe/trunk/lib/Format/CMakeLists.txt
>> >cfe/trunk/lib/Format/Format.cpp
>> >cfe/trunk/lib/Tooling/CMakeLists.txt
>> >cfe/trunk/lib/Tooling/Core/CMakeLists.txt
>> >cfe/trunk/unittests/Tooling/HeaderIncludesTest.cpp
>>

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

2018-05-18 Thread Amara Emerson via cfe-commits 
Hi Eric,

Green dragon buildbots have started failing too, e.g.: 
http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/10222/

If you don’t have a quick fix can you please revert it.

Thanks,
Amara

> On May 18, 2018, at 7:46 PM, Eric Liu  wrote:
> 
> Hi Vedant,
> 
> It seems that your build was not using cmake files in the source tree? 
> lib/Tooling/Inclusions/ is a (new) standalone library 
> (clangToolingInclusions, similar to clangToolingCore). You might need update 
> your build to reflect this change. Let me know if you have any further 
> question.
> 
> Thanks,
> Eric
> 
> On Fri, May 18, 2018 at 6:41 PM Vedant Kumar  > wrote:
> Hi Eric,
> 
> I think there might be a cyclic dependency issue here, possibly one that's 
> only visible with LLVM_ENABLE_MODULES=On. I see:
> 
> While building module 'Clang_Tooling' imported from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10: 
> 
> While building module 'Clang_Format' imported from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:
>
> In file included from :1:
>   
>  
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Format/Format.h:20:10:
>  fatal error: cyclic dependency in module 'Clang_Tooling': Clang_Tooling $> 
> Clang_Format -> Clang_Tooling 
>
> #include "clang/Tooling/Inclusions/IncludeStyle.h"
>  ^  
> While building module 'Clang_Tooling' imported from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
> In file included from :22:   
>
> In file included from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringAction.h:14:
> In file included from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRules.h:14:
> In file included from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h:16:
> In file included from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:14:
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:10:
>  fatal error: could not build module 'Clang_Format'
> #include "clang/Format/Format.h"
>  ^~~
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:10:
>  fatal error: could not build module 'Clang_Tooling'
> #include "clang/Tooling/Execution.h"  
>   
>  ^~~
> 3 errors generated.
> 
> Could you take a look?
> 
> thanks,
> vedant
> 
> > On May 18, 2018, at 7:16 AM, Eric Liu via cfe-commits 
> > > wrote:
> > 
> > Author: ioeric
> > Date: Fri May 18 07:16:37 2018
> > New Revision: 332720
> > 
> > URL: http://llvm.org/viewvc/llvm-project?rev=332720=rev 
> > 
> > Log:
> > Move #include manipulation code to new lib/Tooling/Inclusions.
> > 
> > Summary:
> > clangToolingCore is linked into almost everything (incl. clang), but
> > not few tools need #include manipulation at this point. So pull this into a
> > separate library in Tooling.
> > 
> > Reviewers: ilya-biryukov
> > 
> > Subscribers: klimek, mgorny, cfe-commits, thakis
> > 
> > Differential Revision: https://reviews.llvm.org/D47068 
> > 
> > 
> > Added:
> >cfe/trunk/include/clang/Tooling/Inclusions/
> >cfe/trunk/include/clang/Tooling/Inclusions/HeaderIncludes.h
> >  - copied, changed from r332717, 
> > cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
> >cfe/trunk/include/clang/Tooling/Inclusions/IncludeStyle.h
> >  - copied, changed from r332717, 
> > cfe/trunk/include/clang/Tooling/Core/IncludeStyle.h
> >cfe/trunk/lib/Tooling/Inclusions/
> >cfe/trunk/lib/Tooling/Inclusions/CMakeLists.txt
> >  - copied, changed from r332717, 
> > cfe/trunk/lib/Tooling/Core/CMakeLists.txt
> >cfe/trunk/lib/Tooling/Inclusions/HeaderIncludes.cpp
> >  - copied, changed from r332717, 
> > cfe/trunk/lib/Tooling/Core/HeaderIncludes.cpp
> >cfe/trunk/lib/Tooling/Inclusions/IncludeStyle.cpp
> >  - copied, changed from r332717, 
> > cfe/trunk/lib/Tooling/Core/IncludeStyle.cpp
> > Removed:
> >cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
> >cfe/trunk/include/clang/Tooling/Core/IncludeStyle.h
> >cfe/trunk/lib/Tooling/Core/HeaderIncludes.cpp
> >

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

2018-05-18 Thread Vedant Kumar via cfe-commits 
I wiped my build directory, updated to r332734 and rebuilt, but hit the same 
error while building check-clang.

For completeness, I did:

$ cmake -G Ninja /Users/vsk/src/llvm.org-master/llvm -DCMAKE_BUILD_TYPE=Release 
-DLLVM_ENABLE_ASSERTIONS=On -DCLANG_ENABLE_ARCMT=Off -DCLANG_
ENABLE_STATIC_ANALYZER=Off -DLLVM_TARGETS_TO_BUILD="X86;ARM;AArch64" 
-DLLVM_ENABLE_MODULES=On
$ ninja check-clang

Are you able to reproduce the issue?

vedant

> On May 18, 2018, at 9:46 AM, Eric Liu  wrote:
> 
> Hi Vedant,
> 
> It seems that your build was not using cmake files in the source tree? 
> lib/Tooling/Inclusions/ is a (new) standalone library 
> (clangToolingInclusions, similar to clangToolingCore). You might need update 
> your build to reflect this change. Let me know if you have any further 
> question.
> 
> Thanks,
> Eric
> 
> On Fri, May 18, 2018 at 6:41 PM Vedant Kumar  wrote:
> Hi Eric,
> 
> I think there might be a cyclic dependency issue here, possibly one that's 
> only visible with LLVM_ENABLE_MODULES=On. I see:
> 
> While building module 'Clang_Tooling' imported from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10: 
> 
> While building module 'Clang_Format' imported from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:
>
> In file included from :1:
>   
>  
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Format/Format.h:20:10:
>  fatal error: cyclic dependency in module 'Clang_Tooling': Clang_Tooling $> 
> Clang_Format -> Clang_Tooling 
>
> #include "clang/Tooling/Inclusions/IncludeStyle.h"
>  ^  
> While building module 'Clang_Tooling' imported from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
> In file included from :22:   
>
> In file included from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringAction.h:14:
> In file included from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRules.h:14:
> In file included from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h:16:
> In file included from 
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:14:
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:10:
>  fatal error: could not build module 'Clang_Format'
> #include "clang/Format/Format.h"
>  ^~~
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:10:
>  fatal error: could not build module 'Clang_Tooling'
> #include "clang/Tooling/Execution.h"  
>   
>  ^~~
> 3 errors generated.
> 
> Could you take a look?
> 
> thanks,
> vedant
> 
> > On May 18, 2018, at 7:16 AM, Eric Liu via cfe-commits 
> >  wrote:
> > 
> > Author: ioeric
> > Date: Fri May 18 07:16:37 2018
> > New Revision: 332720
> > 
> > URL: http://llvm.org/viewvc/llvm-project?rev=332720=rev
> > Log:
> > Move #include manipulation code to new lib/Tooling/Inclusions.
> > 
> > Summary:
> > clangToolingCore is linked into almost everything (incl. clang), but
> > not few tools need #include manipulation at this point. So pull this into a
> > separate library in Tooling.
> > 
> > Reviewers: ilya-biryukov
> > 
> > Subscribers: klimek, mgorny, cfe-commits, thakis
> > 
> > Differential Revision: https://reviews.llvm.org/D47068
> > 
> > Added:
> >cfe/trunk/include/clang/Tooling/Inclusions/
> >cfe/trunk/include/clang/Tooling/Inclusions/HeaderIncludes.h
> >  - copied, changed from r332717, 
> > cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
> >cfe/trunk/include/clang/Tooling/Inclusions/IncludeStyle.h
> >  - copied, changed from r332717, 
> > cfe/trunk/include/clang/Tooling/Core/IncludeStyle.h
> >cfe/trunk/lib/Tooling/Inclusions/
> >cfe/trunk/lib/Tooling/Inclusions/CMakeLists.txt
> >  - copied, changed from r332717, 
> > cfe/trunk/lib/Tooling/Core/CMakeLists.txt
> >cfe/trunk/lib/Tooling/Inclusions/HeaderIncludes.cpp
> >  - copied, changed from r332717, 
> > cfe/trunk/lib/Tooling/Core/HeaderIncludes.cpp
> >cfe/trunk/lib/Tooling/Inclusions/IncludeStyle.cpp
> >  - copied, changed from r332717, 
> > cfe/trunk/lib/Tooling/Core/IncludeStyle.cpp
> > Removed:
> >cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
> >

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

2018-05-18 Thread Eric Liu via cfe-commits 
Hi Vedant,

It seems that your build was not using cmake files in the source tree?
lib/Tooling/Inclusions/
is a (new) standalone library (clangToolingInclusions, similar to
clangToolingCore). You might need update your build to reflect this change.
Let me know if you have any further question.

Thanks,
Eric

On Fri, May 18, 2018 at 6:41 PM Vedant Kumar  wrote:

> Hi Eric,
>
> I think there might be a cyclic dependency issue here, possibly one that's
> only visible with LLVM_ENABLE_MODULES=On. I see:
>
> While building module 'Clang_Tooling' imported from
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
>
> While building module 'Clang_Format' imported from
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:
>
> In file included from :1:
>
>
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Format/Format.h:20:10:
> fatal error: cyclic dependency in module 'Clang_Tooling': Clang_Tooling $>
> Clang_Format -> Clang_Tooling
>
> #include "clang/Tooling/Inclusions/IncludeStyle.h"
>  ^
> While building module 'Clang_Tooling' imported from
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
> In file included from :22:
>
> In file included from
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringAction.h:14:
> In file included from
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRules.h:14:
> In file included from
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h:16:
> In file included from
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:14:
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:10:
> fatal error: could not build module 'Clang_Format'
> #include "clang/Format/Format.h"
>  ^~~
> /Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:10:
> fatal error: could not build module 'Clang_Tooling'
> #include "clang/Tooling/Execution.h"
>
>  ^~~
> 3 errors generated.
>
> Could you take a look?
>
> thanks,
> vedant
>
> > On May 18, 2018, at 7:16 AM, Eric Liu via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Author: ioeric
> > Date: Fri May 18 07:16:37 2018
> > New Revision: 332720
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=332720=rev
> > Log:
> > Move #include manipulation code to new lib/Tooling/Inclusions.
> >
> > Summary:
> > clangToolingCore is linked into almost everything (incl. clang), but
> > not few tools need #include manipulation at this point. So pull this
> into a
> > separate library in Tooling.
> >
> > Reviewers: ilya-biryukov
> >
> > Subscribers: klimek, mgorny, cfe-commits, thakis
> >
> > Differential Revision: https://reviews.llvm.org/D47068
> >
> > Added:
> >cfe/trunk/include/clang/Tooling/Inclusions/
> >cfe/trunk/include/clang/Tooling/Inclusions/HeaderIncludes.h
> >  - copied, changed from r332717,
> cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
> >cfe/trunk/include/clang/Tooling/Inclusions/IncludeStyle.h
> >  - copied, changed from r332717,
> cfe/trunk/include/clang/Tooling/Core/IncludeStyle.h
> >cfe/trunk/lib/Tooling/Inclusions/
> >cfe/trunk/lib/Tooling/Inclusions/CMakeLists.txt
> >  - copied, changed from r332717,
> cfe/trunk/lib/Tooling/Core/CMakeLists.txt
> >cfe/trunk/lib/Tooling/Inclusions/HeaderIncludes.cpp
> >  - copied, changed from r332717,
> cfe/trunk/lib/Tooling/Core/HeaderIncludes.cpp
> >cfe/trunk/lib/Tooling/Inclusions/IncludeStyle.cpp
> >  - copied, changed from r332717,
> cfe/trunk/lib/Tooling/Core/IncludeStyle.cpp
> > Removed:
> >cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
> >cfe/trunk/include/clang/Tooling/Core/IncludeStyle.h
> >cfe/trunk/lib/Tooling/Core/HeaderIncludes.cpp
> >cfe/trunk/lib/Tooling/Core/IncludeStyle.cpp
> > Modified:
> >cfe/trunk/include/clang/Format/Format.h
> >cfe/trunk/lib/Format/CMakeLists.txt
> >cfe/trunk/lib/Format/Format.cpp
> >cfe/trunk/lib/Tooling/CMakeLists.txt
> >cfe/trunk/lib/Tooling/Core/CMakeLists.txt
> >cfe/trunk/unittests/Tooling/HeaderIncludesTest.cpp
> >
> > Modified: cfe/trunk/include/clang/Format/Format.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=332720=332719=332720=diff
> >
> ==
> > --- cfe/trunk/include/clang/Format/Format.h (original)
> > +++ cfe/trunk/include/clang/Format/Format.h Fri May 18 07:16:37 2018
> > @@ -16,8 +16,8 @@
> > #define LLVM_CLANG_FORMAT_FORMAT_H
> >
> > #include "clang/Basic/LangOptions.h"
> > -#include "clang/Tooling/Core/IncludeStyle.h"
> > #include

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

2018-05-18 Thread Vedant Kumar via cfe-commits 
Hi Eric,

I think there might be a cyclic dependency issue here, possibly one that's only 
visible with LLVM_ENABLE_MODULES=On. I see:

While building module 'Clang_Tooling' imported from 
/Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:   
  
While building module 'Clang_Format' imported from 
/Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:
   
In file included from :1:  
 
/Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Format/Format.h:20:10:
 fatal error: cyclic dependency in module 'Clang_Tooling': Clang_Tooling $> 
Clang_Format -> Clang_Tooling   
 
#include "clang/Tooling/Inclusions/IncludeStyle.h"
 ^  
While building module 'Clang_Tooling' imported from 
/Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:
In file included from :22: 
 
In file included from 
/Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringAction.h:14:
In file included from 
/Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRules.h:14:
In file included from 
/Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h:16:
In file included from 
/Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/RefactoringResultConsumer.h:14:
/Users/vsk/src/llvm.org-master/llvm/tools/clang/include/clang/Tooling/Refactoring/AtomicChange.h:19:10:
 fatal error: could not build module 'Clang_Format'
#include "clang/Format/Format.h"
 ^~~
/Users/vsk/src/llvm.org-master/llvm/tools/clang/lib/Tooling/Execution.cpp:10:10:
 fatal error: could not build module 'Clang_Tooling'
#include "clang/Tooling/Execution.h"
 ^~~
3 errors generated.

Could you take a look?

thanks,
vedant

> On May 18, 2018, at 7:16 AM, Eric Liu via cfe-commits 
>  wrote:
> 
> Author: ioeric
> Date: Fri May 18 07:16:37 2018
> New Revision: 332720
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=332720=rev
> Log:
> Move #include manipulation code to new lib/Tooling/Inclusions.
> 
> Summary:
> clangToolingCore is linked into almost everything (incl. clang), but
> not few tools need #include manipulation at this point. So pull this into a
> separate library in Tooling.
> 
> Reviewers: ilya-biryukov
> 
> Subscribers: klimek, mgorny, cfe-commits, thakis
> 
> Differential Revision: https://reviews.llvm.org/D47068
> 
> Added:
>cfe/trunk/include/clang/Tooling/Inclusions/
>cfe/trunk/include/clang/Tooling/Inclusions/HeaderIncludes.h
>  - copied, changed from r332717, 
> cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
>cfe/trunk/include/clang/Tooling/Inclusions/IncludeStyle.h
>  - copied, changed from r332717, 
> cfe/trunk/include/clang/Tooling/Core/IncludeStyle.h
>cfe/trunk/lib/Tooling/Inclusions/
>cfe/trunk/lib/Tooling/Inclusions/CMakeLists.txt
>  - copied, changed from r332717, cfe/trunk/lib/Tooling/Core/CMakeLists.txt
>cfe/trunk/lib/Tooling/Inclusions/HeaderIncludes.cpp
>  - copied, changed from r332717, 
> cfe/trunk/lib/Tooling/Core/HeaderIncludes.cpp
>cfe/trunk/lib/Tooling/Inclusions/IncludeStyle.cpp
>  - copied, changed from r332717, 
> cfe/trunk/lib/Tooling/Core/IncludeStyle.cpp
> Removed:
>cfe/trunk/include/clang/Tooling/Core/HeaderIncludes.h
>cfe/trunk/include/clang/Tooling/Core/IncludeStyle.h
>cfe/trunk/lib/Tooling/Core/HeaderIncludes.cpp
>cfe/trunk/lib/Tooling/Core/IncludeStyle.cpp
> Modified:
>cfe/trunk/include/clang/Format/Format.h
>cfe/trunk/lib/Format/CMakeLists.txt
>cfe/trunk/lib/Format/Format.cpp
>cfe/trunk/lib/Tooling/CMakeLists.txt
>cfe/trunk/lib/Tooling/Core/CMakeLists.txt
>cfe/trunk/unittests/Tooling/HeaderIncludesTest.cpp
> 
> Modified: cfe/trunk/include/clang/Format/Format.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=332720=332719=332720=diff
> ==
> --- cfe/trunk/include/clang/Format/Format.h (original)
> +++ cfe/trunk/include/clang/Format/Format.h Fri May 18 07:16:37 2018
> @@ -16,8 +16,8 @@
> #define LLVM_CLANG_FORMAT_FORMAT_H
> 
> #include "clang/Basic/LangOptions.h"
> -#include "clang/Tooling/Core/IncludeStyle.h"
> #include "clang/Tooling/Core/Replacement.h"
> +#include "clang/Tooling/Inclusions/IncludeStyle.h"
> #include "llvm/ADT/ArrayRef.h"
> #include "llvm/Support/Regex.h"
> #include 
>