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

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

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

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

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

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

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

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,

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

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

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_

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.

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:

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

2018-05-18 Thread Eric Liu via cfe-commits
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