[cfe-commits] r160265 - in /cfe/trunk: include/clang/Tooling/CommandLineClangTool.h lib/Tooling/CMakeLists.txt lib/Tooling/CommandLineClangTool.cpp test/CMakeLists.txt test/Tooling/clang-ast-dump.cpp tools/CMakeLists.txt tools/Makefile tools/clan

[Eli Friedman]

On Mon, Jul 16, 2012 at 11:56 PM, Manuel Klimek <klimek at google.com> wrote:
> On Tue, Jul 17, 2012 at 7:22 AM, Douglas Gregor <dgregor at apple.com> wrote:
>>
>> On Jul 16, 2012, at 5:46 AM, Alexander Kornienko <alexfh at google.com> wrote:
>>
>>> Author: alexfh
>>> Date: Mon Jul 16 07:46:48 2012
>>> New Revision: 160265
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=160265&view=rev
>>> Log:
>>> The new clang-ast-dump tool for selective AST dumping. Moved common command-line tool stuff to CommandLineClangTool
>>
>> I don't think this belongs in the Clang repository because:
>>
>>         1) It's built on a *debugging* option, the XML dumper, which is neither actively maintained nor complete enough to build any tool upon
>>         2) It provides no value for users of Clang
>>         3) It provides very little value for developers of Clang tools, since it does little more than clang + grep
>>         4) It fails nearly all of the criteria for a Clang extension outlined in http://clang.llvm.org/get_involved.html
>>
>> I'll be happy to hear opinions to the contrary (Manuel?), but barring some killer argument for this feature, I'd like it reverted.
>
> I'll provide an argument for why I think this belongs in clang
> mainline, but it's also no sweat to pull it into the (hopefully soon
> available) tooling repo.
>
> There are 2 killer features for this vs. clang -ast-dump-xml:
> - clang -ast-dump-xml + grep doesn't scale: outputting a whole AST in
> text often produces > 20GB of output and takes a long, long time -
> grepping just on the node names is really fast
> - clang -ast-dump-xml requires the user to first get a correct -cc1
> invocation, which is non-trivial for people who just write tools
>
> Overall, a way to dump the AST is critical to writing clang tools,
> especially while you're learning the AST. Being able to dump parts of
> the AST of the source you want to work on makes it far easier to find
> out why the AST looks the way it does at a certain place where your
> refactoring doesn't work than trying to reproduce that with a minimal
> source file that doesn't use any non-builtin includes. Many tool
> writers are not hardcore C++ experts (the ones that are obviously
> don't need the tool that much), and one of the goals for us is to make
> writing C++ tools possible for normal people ;)
>
> One thing that obviously needs to be addressed is what to do in
> release mode. Independently of where we put this tool, I think having
> a supported (we're going to support it ;) version of an ASTDumpXML
> frontend action that works in release mode is important to us, so we
> can build a tool and release it to our internal customers by default
> in a release build. My proposal would be to change the builtin clang
> version to completely not depend on it in release mode, as Alex
> pointed out. Open to other ideas, though.
>
> Please advice in how to proceed. As I said, migrating this into the
> new repo is easy enough. Also, any changes you want to see to make it
> more in line with the extension policy would be interesting for me, so
> I can make better decisions on changes like this in the future (I
> think most of the policy points are met, or are meant to be met (minus
> us screwing up, but that's why we have reviews :), or not applicable).

I suppose a developer using the tooling infrastructure might want some
sort of AST dump, but how close is the current ASTDumpXML to what you
actually want?  It seems like for debugging purposes, you'd want a
tree annotated with things like what kinds of matchers would match a
given construct, not a pure dump of the internal tree.

-Eli