[cfe-commits] Comment AST and parser

[Dmitri Gribenko]

On Mon, Jul 2, 2012 at 9:39 AM, Douglas Gregor <dgregor at apple.com> wrote:
> This is looking great! I like this direction a lot. Some specific comments below.

Thank you for looking!  Answers to discussion questions follow.

> +/// A newline.
> +class NewlineComment : public InlineContentComment {
> +public:
>
> Why have a special AST node for newlines, rather than either embedding \n's or having a "there's a newline following this inline comment" bit? Abstractly, I guess having an AST node to cope with might make it easier for clients of the comment AST to translate it properly, or is there another reason?

Information about newlines can be used to correctly display HTML tag
<pre>.  (But we have verbatim blocks, so why use <pre>?)  I don't know
if it will be useful for anything except that.  The parser can just
silently drop newlines.   I thought that it would be easier to remove
newline nodes if we consider them useless than to introduce them --
that's the only reason I added this AST node.

Turning a newline node into a bit on inline content seems a good idea,
except that it makes all inline content nodes larger...

> Index: utils/TableGen/TableGen.cpp
> ===================================================================
> --- utils/TableGen/TableGen.cpp (revision 159471)
> +++ utils/TableGen/TableGen.cpp (working copy)
> @@ -38,6 +38,7 @@
>    GenClangDiagsDefs,
>    GenClangDiagGroups,
>    GenClangDiagsIndexName,
> +  GenClangCommentNodes,
>    GenClangDeclNodes,
>    GenClangStmtNodes,
>    GenClangSACheckers,
> @@ -86,6 +87,8 @@
>                      clEnumValN(GenClangDiagsIndexName,
>                                 "gen-clang-diags-index-name",
>                                 "Generate Clang diagnostic name index"),
> +                    clEnumValN(GenClangCommentNodes, "gen-clang-comment-nodes",
> +                               "Generate Clang AST comment nodes"),
>                      clEnumValN(GenClangDeclNodes, "gen-clang-decl-nodes",
>                                 "Generate Clang AST declaration nodes"),
>                      clEnumValN(GenClangStmtNodes, "gen-clang-stmt-nodes",
> @@ -148,6 +151,9 @@
>      case GenClangDiagsIndexName:
>        EmitClangDiagsIndexName(Records, OS);
>        break;
> +    case GenClangCommentNodes:
> +      EmitClangASTNodes(Records, OS, "Comment", "");
> +      break;
>      case GenClangDeclNodes:
>        EmitClangASTNodes(Records, OS, "Decl", "Decl");
>        EmitClangDeclContext(Records, OS);
>
> Is there more that you intend to do with TableGen for the AST nodes themselves? It looks like all we're getting is the list of node names, but TableGen is a pretty heavyweight way to keep that up-to-date.

Currently we tablegen'ing clang/AST/CommentNodes.inc.  I plan to
tablegen a list of supported commands with two attributes: command
kind (inline/block/verbatim block/verbatim line) and number of
arguments.

Or did you mean something particular that I didn't notice?

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/