Hi Doug,<div><br></div><div>any chance I could get this reviewed? I have another feature for the compilation database (correctly handling relative directories) that I would like to implement in the new structure.</div><div>
<br></div><div>Cheers,<br>Daniel</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 23, 2012 at 1:11 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Any thoughts on this design?<div>Cheers,<br>Daniel</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br>
<br><div class="gmail_quote">On Fri, Jul 20, 2012 at 2:11 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I moved the class-comment back and properly propagated the error instead of printing it to outs(). I don't know about using Diagnostics. Could we get to that in a different patch to clearly separate those changes?<div>
<br>Cheers,</div><div>Daniel</div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jul 20, 2012 at 12:41 PM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I'd like the class-comment for JSONCompilationDatabase to still<br>
include what's now in the file comment, so it's visible in doxygen.<br>
<br>
Also, I'd prefer to use a result value to capture the error message<br>
instead of llvm::outs'ing in findCompilationDatabaseFromDirectory.<br>
Perhaps we should also switch this to Diagnostics?<br>
<br>
Cheers,<br>
/Manuel<br>
<div><div><br>
On Fri, Jul 20, 2012 at 12:30 PM, Daniel Jasper <<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>> wrote:<br>
> Attached is a patch to do most of this restructuring, kindly asking for<br>
> review.<br>
><br>
> In particular, it does:<br>
> - Restructure the compilation database architecture to using LLVM's registry<br>
> concept. It should now be possible to link in additional compilation<br>
> databases.<br>
> - Separate the JSONCompilationDatabase from CompilationDatabase to show the<br>
> loose coupling and serve as an example.<br>
><br>
><br>
><br>
><br>
> On Thu, Jul 19, 2012 at 3:17 PM, Stephen Kelly <<a href="mailto:steveire@gmail.com" target="_blank">steveire@gmail.com</a>> wrote:<br>
>><br>
>> On 07/19/2012 01:32 PM, Manuel Klimek wrote:<br>
>> > On Thu, Jul 19, 2012 at 1:10 PM, Stephen Kelly <<a href="mailto:steveire@gmail.com" target="_blank">steveire@gmail.com</a>><br>
>> > wrote:<br>
>> >> Chandler Carruth wrote:<br>
>> >><br>
>> >>> That said, the latest version of CMake already has support for JSON +<br>
>> >>> Ninja -- we didn't contribute it, so I don't know what strategy they<br>
>> >>> followed, but you should look at that and talk to the ninja and CMake<br>
>> >>> developers before going too far here.<br>
>> >>><br>
>> >> I wrote it and pretty much followed the same thing Manuel did in the<br>
>> >> Makefile generator.<br>
>> >><br>
>> >> The commit which actually adds the feature is trivial:<br>
>> >><br>
>> >><br>
>> >> <a href="http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=db839bec7d076b54c5e9ad0d19386a26557a509e" target="_blank">http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=db839bec7d076b54c5e9ad0d19386a26557a509e</a><br>
>> >><br>
>> >> Manuel mentioned before that he'd like to see ninja being able to<br>
>> >> generate a<br>
>> >> database without cmake too though:<br>
>> >><br>
>> >><br>
>> >> <a href="http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3678/focus=3697" target="_blank">http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/3678/focus=3697</a><br>
>> >><br>
>> >> As Chandler said, it's not in a release yet, but will be in the next<br>
>> >> release<br>
>> >> in a few weeks. Feel free to test the release candidate (I would<br>
>> >> appreciate<br>
>> >> if you did)<br>
>> > I know multiple people who are refusing to work without this any more,<br>
>> > I've been using it since it landed in "next". So consider this part<br>
>> > pretty well tested (at least on the llvm codebase).<br>
>> ><br>
>><br>
>> Good to hear :)<br>
>><br>
>> Thanks,<br>
>><br>
>> Steve.<br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
><br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>