<div dir="ltr">Thanks a lot for looking into this Bruno! The fix looks promising; I'll give it a try next week :D<div><br><div class="gmail_quote"><div dir="ltr">On Fri, May 18, 2018 at 10:33 PM David Blaikie via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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).</div></blockquote><div>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.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"></div><br><div class="gmail_quote"></div><div class="gmail_quote"><div dir="ltr">On Fri, May 18, 2018 at 1:10 PM Bruno Cardoso Lopes via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, May 18, 2018 at 12:46 PM Bruno Cardoso Lopes <<a href="mailto:bruno.cardoso@gmail.com" target="_blank">bruno.cardoso@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, May 18, 2018 at 11:54 AM Vedant Kumar via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><blockquote type="cite">On May 18, 2018, at 11:48 AM, Eric Liu <<a href="mailto:ioeric@google.com" target="_blank">ioeric@google.com</a>> wrote:<br></blockquote><div><blockquote type="cite"><br class="m_-7626407748062982014m_8172054451203383588gmail-m_-3115138368819719797Apple-interchange-newline"><div><div dir="ltr">So I have reverted this with r<span style="color:rgb(33,33,33)">332751</span><span style="color:rgb(33,33,33)">.</span></div></div></blockquote><div><br></div>Thanks!</div><div><br></div><div><br><blockquote type="cite"><div><div dir="ltr"><div><font color="#212121">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 </font><span style="color:rgb(33,33,33)">clangTooling and </span><span style="color:rgb(33,33,33)">clangToolingInclusions... I'd appreciate it if someone who knows how clang module build is set up could help take a look.</span></div></div></div></blockquote><div><br></div><div>+ Bruno & David who have more experience in this area than I do.</div></div></div></blockquote><div><br></div><div>Gonna try to reproduce and take a look!</div></div></div></blockquote><div><br></div><div>I could reproduce it. You should be good to go if you add another top level module for Inclusions (and break the dep):</div><div><br></div><div><div>--- a/include/clang/module.modulemap</div><div>+++ b/include/clang/module.modulemap</div><div>@@ -153,3 +153,8 @@ module Clang_ToolingCore {</div><div>   requires cplusplus</div><div>   umbrella "Tooling/Core" module * { export * }</div><div> }</div><div>+</div><div>+module Clang_ToolingInclusions {</div><div>+  requires cplusplus</div><div>+  umbrella "Tooling/Inclusions" module * { export * }</div><div>+}</div></div></div></div><div dir="ltr"><div><br></div>-- <br><div dir="ltr" class="m_-7626407748062982014m_8172054451203383588gmail_signature">Bruno Cardoso Lopes <br><a href="http://www.brunocardoso.cc" target="_blank">http://www.brunocardoso.cc</a></div></div>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div>