[cfe-commits] [PATCH] Add support for pretty-printing attributes

[Richard Membarth]

On 2011.11.07 20:00, Douglas Gregor wrote:
> 
> On Nov 4, 2011, at 7:44 AM, Richard Membarth wrote:
> 
> > Attached is a patch that adds support for pretty-printing attributes
> > stored in the AST. Tblgen is used to generate the necessary code for
> > pretty printing attributes as suggest by Peter in [1].
> > 
> > Please let me know if this is ok.
> 
> 
> This generally looks good. My only non-trivial comment is that this printing assumes that all of the attributes can be pretty-printed by just printing the arguments as a comma-separated list, but I doubt that's actually the case. If nothing else, the printing of VersionArguments looks incorrect for the "availability" attribute. It would be better to have some way to escape out to hand-written printing code, and then implement that code for the cases where the default comma-separated list printing doesn't work.

I looked into this today and updated the patch, which is attached to
this mail. Printing of VersionArguments was indeed not correct. I
added a check to call a separate hand-written pretty printing
function for the "availability" attribute. The same can be done in
case other attributes need a similar special handling.
I hope this is now sufficient for pretty printing the currently
available C/C++ attributes.

> 
> One minor thing:
> 
> @@ -362,6 +382,18 @@ namespace {
>           << getLowerName() << "_end(); i != e; ++i)\n";
>        OS << "      " << WritePCHRecord(type, "(*i)");
>      }
> +    void writeValue(raw_ostream &OS) const {
> +      OS << "\";\n";
> +      OS << "  bool isFirst = true;\n"
> +         << "  for (" << getAttrName() << "Attr::" << getLowerName()
> +         << "_iterator i = " << getLowerName() << "_begin(), e = "
> +         << getLowerName() << "_end(); i != e; ++i) {\n"
> +         << "    if (!isFirst) isFirst = false;\n"
> +         << "    else OS << \", \";\n"
> 
> Shouldn't this be if(isFirst)?

Yes, this is also updated in the patch.

Richard

> 
> +         << "    OS << *i;\n"
> +         << "  }\n";
> +      OS << "  OS << \"";
> +    }
>    };
> 
> 
> 
> 	- Doug

-------------- next part --------------
A non-text attachment was scrubbed...
Name: prettyprint_attributes.patch
Type: text/x-diff
Size: 8728 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111111/2dcac540/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111111/2dcac540/attachment.sig>