+dgregor<div><br></div><div><div>On Mon, Sep 24, 2012 at 5:02 AM, Philip Craig <span dir="ltr"><<a href="mailto:philipjcraig@gmail.com" target="_blank">philipjcraig@gmail.com</a>></span> wrote:<br><div><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Sat, Sep 22, 2012 at 7:00 AM, Philip Craig <<a href="mailto:philipjcraig@gmail.com">philipjcraig@gmail.com</a>> wrote:<br>

> On Fri, Sep 21, 2012 at 11:21 PM, Alexander Kornienko <<a href="mailto:alexfh@google.com">alexfh@google.com</a>> wrote:<br>
>><br>
>> On Fri, Sep 21, 2012 at 2:40 PM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br>
>>><br>
>>> On Fri, Sep 21, 2012 at 12:32 PM, Philip Craig <<a href="mailto:philipjcraig@gmail.com">philipjcraig@gmail.com</a>><br>
>>> wrote:<br>
>>> > On Fri, Sep 21, 2012 at 7:49 PM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>><br>
>>> > wrote:<br>
>>> >> Hi Philip,<br>
>>> >><br>
>>> >> we had a discussion around the strategy for handling ast dumping<br>
>>> >> fairly recently. I think the common agreement in the end was that<br>
>>> >> instead of having an extra tool, we want to make clang's -ast-dump<br>
>>> >> awesome.<br>
>>> ><br>
>>> > Was this discussion on the mailing list? Any chance you could point me<br>
>>> > to it?<br>
>>><br>
>>><br>
>>> <a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120716/060831.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120716/060831.html</a><br>
><br>
> Thanks.<br>
><br>
>>> >> I don't know what your ultimate goal is here, but from my<br>
>>> >> side any work that goes towards making clang -ast-dump better would be<br>
>>> >> highly appreciated :)<br>
>>> ><br>
>>> > My motivation was having some way of getting the structure of the AST<br>
>>> > for a given piece of source code, so that it is easier to correctly<br>
>>> > call the AST matchers to match that code. The issue with clang<br>
>>> > -ast-dump is that it pretty prints decls and types. So would the goal<br>
>>> > here be to add more command line options to clang to control how<br>
>>> > -ast-dump prints the AST?<br>
>>><br>
>>> No, I think the goal is to dump decls and types in a sensible way :)<br>
>><br>
>><br>
>> More specifically, I think we agreed on changing -ast-dump from<br>
>> pretty-printing declarations to outputting them in the same LISP-style<br>
>> format, which is used for statements. I was going to implement that, but<br>
>> never had time to do this. If you're going to continue work on your utility,<br>
>> it would be much more valuable, if you instead improved clang's current<br>
>> -ast-dump option. In that case you consider to go this way, here's where to<br>
>> start:<br>
>><br>
>> clang/lib/Frontend/ASTConsumers.cpp:120: ASTConsumer<br>
>> *clang::CreateASTDumper(StringRef FilterString)<br>
>><br>
>> is a common entry point, used by "clang -cc1 -ast-dump" and by "clang-check<br>
>> -ast-dump". An additional benefit from this would be that after "-ast-dump"<br>
>> starts outputting the AST for declarations in a structured form,<br>
>> "-ast-dump-xml" will become useless and can be removed.<br>
><br>
> I'll have a look into that and get an idea of what's involved.<br>
<br>
</div></div>I'm going to try working on this. Here's my thoughts on how to structure this.<br>
<br>
Currently, it looks like this (I'm ignoring types for now):<br>
<br>
CreateASTDumper calls either Decl::print() or Decl::dump().<br>
<br>
Both Decl::print() and Decl::dump() use class DeclPrinter. The only<br>
difference between these uses is that Decl::dump() passes a<br>
PrintingPolicy that has DumpSourceManager set.<br>
<br>
When DeclPrinter encounters a statement, it calls Stmt::printPretty().<br>
If DumpSourceManger is set, then Stmt::printPretty() calls<br>
Stmt::dump(), otherwise it uses class StmtPrinter.<br>
<br>
Stmt::dump() uses class StmtDumper.<br>
<br>
<br>
My proposed changes are:<br>
<br>
I will change Stmt::printPretty() to never call Stmt::dump(). This<br>
also removes the need for the DumpSourceManager field in<br>
PrintingPolicy.<br>
<br>
I will create a new DeclDumper class, which is called by Decl::dump(),<br>
and which calls Stmt::dump() when it encounters a statement.<br>
<br>
There will be some duplication of functionality and state between<br>
StmtDumper and DeclDumper. So I will extract parts of StmtDumper out<br>
into an ASTDumper class. This will result in a design that is very<br>
similar to the ASTWriter for serialization.<br></blockquote><div><br></div><div>As for me, this looks fine, but I'm relatively new to Clang, so I can easily miss something. It would be great, if Doug looked at this.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
My first patch will be to create the ASTDumper class. Later patches<br>
will create DeclDumper. For the creation of the DeclDumper class,<br>
should I do this as one big patch, or should I try to find a way to<br>
break it into smaller patches?<br></blockquote><div><br></div><div>As long as you can split this into self-contained patches, which have value on their own, please do. But if your changes will only have value together, I guess they are better submitted as one patch. Doug, am I right here?</div>
<div><br></div><div>Anyway, you can post your patches as you go, so you can get feedback earlier. I would also strongly suggest using our code review tool to submit patches: <a href="http://llvm-reviews.chandlerc.com/">http://llvm-reviews.chandlerc.com/</a></div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">>>><br>
>>> > Are there any other areas where you think -ast-dump needs to be better?<br>
>>><br>
>>> As you said, decls and types need to be structured nicely. Also, a<br>
>>> while ago Richard proposed a patch to add coloring, no idea how far he<br>
>>> got (cc'ing him)<br>
>>><br>
>>> Cheers,<br>
>>> /Manuel<br>
>>><br>
>>> ><br>
>>> >><br>
>>> >> Cheers,<br>
>>> >> /Manuel<br>
>>> >><br>
>>> >> On Fri, Sep 21, 2012 at 1:17 AM, Philip Craig <<a href="mailto:philipjcraig@gmail.com">philipjcraig@gmail.com</a>><br>
>>> >> wrote:<br>
>>> >>> Hi,<br>
>>> >>><br>
>>> >>> I've been writing a tool to print a clang AST. You can find it at<br>
>>> >>> <a href="https://github.com/philipc/clang-ast" target="_blank">https://github.com/philipc/clang-ast</a>. I've been mostly writing this as<br>
>>> >>> a learning exercise, but I would like to see if there is any wider<br>
>>> >>> interest in the tool.<br>
>>> >>><br>
>>> >>> I'm new to clang, and interested in working on tooling. For this task,<br>
>>> >>> it is important to have an understanding of the AST. I've found the<br>
>>> >>> documentation at <a href="http://clang.llvm.org/docs/InternalsManual.html" target="_blank">http://clang.llvm.org/docs/InternalsManual.html</a> and<br>
>>> >>> <a href="http://clang.llvm.org/docs/IntroductionToTheClangAST.html" target="_blank">http://clang.llvm.org/docs/IntroductionToTheClangAST.html</a> (is there<br>
>>> >>> any more?), but I think it would be helpful to be able to print the<br>
>>> >>> AST for source code to see how it is used in practice. I've found two<br>
>>> >>> ways to do this currently, which aren't quite what I wanted:<br>
>>> >>><br>
>>> >>> clang --ast-dump<br>
>>> >>> - pretty prints some parts, has too much internal info, more suited<br>
>>> >>> for debugging use by clang developers<br>
>>> >>><br>
>>> >>> clang --ast-dump-xml<br>
>>> >>> - incomplete and XML is too verbose for this purpose<br>
>>> >>><br>
>>> >>> Since RecursiveASTVisitor seems to be the API that will be used by<br>
>>> >>> tool developers, I've been using RAV to print the AST. I've found a<br>
>>> >>> few minor bugs in RAV as a result of this, which I'm in the process of<br>
>>> >>> submitting.<br>
>>> >>><br>
>>> >>> One limitation of RAV is that the Visit methods aren't given<br>
>>> >>> information about their relationship with their parent, so the tool<br>
>>> >>> can only list all the children of a node, without distinguishing<br>
>>> >>> between the LHS and RHS of an operator, for example. Is there any<br>
>>> >>> desire to extend RAV to be able to do this?<br>
>>> >>><br>
>>> >>> I've been writing small code snippets to test printing the various<br>
>>> >>> parts of the AST. If you want to see examples of the output of the<br>
>>> >>> tool, this is included in the test cases. As a quick example, "void<br>
>>> >>> foo() {}" is printed as:<br>
>>> >>><br>
>>> >>>   FunctionDecl<br>
>>> >>>     DeclarationName foo<br>
>>> >>>     FunctionNoProtoType<br>
>>> >>>       BuiltinType void<br>
>>> >>>     CompoundStmt<br>
>>> >>><br>
>>> >>> Finally, I've been trying to give a textual description of the AST<br>
>>> >>> grammar in <a href="https://github.com/philipc/clang-ast/blob/master/ast.txt" target="_blank">https://github.com/philipc/clang-ast/blob/master/ast.txt</a>.<br>
>>> >>> This is an attempt to give something similar to<br>
>>> >>> <a href="http://docs.python.org/py3k/library/ast.html#abstract-grammar" target="_blank">http://docs.python.org/py3k/library/ast.html#abstract-grammar</a>. I'm not<br>
>>> >>> sure if it is turning out to be that useful though.<br>
>>> >>><br>
>>> >>> Any comments welcome!<br>
>>> >>><br>
>>> >>> Thanks,<br>
>>> >>> Philip<br>
</div></div></blockquote></div><br><br>-- <br>
</div></div><div>Regards,</div><div>Alex</div></div>