On Nov 11, 2011, at 1:25 PM, Richard Membarth wrote:
>
> 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.
Excellent, thanks! Committed as r145002.
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
