[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

[Manuel Klimek]

On Tue, Jul 17, 2012 at 10:01 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
> 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.

I think those are 2 orthogonal problems - one is for debugging
matchers, the other is for learning the clang AST - I'm working on a
daily basis with people who "see" the clang AST for the first time,
and it's hard to make a mental model of it. Having a good tool to dump
the parts that you're currently looking at to learn how things are
built on top of each other is very important, and the ast-dump-xml
helps a lot.

I also want better debugging for matchers in the future, but that's a
much more complex problem, especially in how to visualize it, and I
think it'll not give you the same overview of how the AST works.

Cheers,
/Manuel