[cfe-commits] [Patch] PR14922 - Improve Attr::printPretty

[Michael Han]

Cool thanks. Please take a look at attached patch, a brief description :
- Introduce a new bit field in Attr to store the index into the spelling
list of each attribute in Attr.td.
- Introduce a new method to AttributeList to generate the index based on
attribute syntax, name, and scope.
  Implementation of this method is generated through table-gen.
- Teach table-gen to print an attribute based on this index.

Michael

On Mon, Jan 14, 2013 at 6:55 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> On Mon, Jan 14, 2013 at 11:40 AM, Michael Han <fragmentshaders at gmail.com>wrote:
>
>> Hi Richard,
>>
>> I think that is a great idea but I am not sure what value to pass to Attr
>> when it's constructed in SemaDeclAttr.cpp. I was hoping to reuse the syntax
>> enumerator value as the spelling list index but it does not work for all
>> cases (e.g. alignment attribute in GNU syntax has two spellings.).
>>
>> Alternatively, at the time we construct an Attr, there is enough
>> information (both the syntax and the actual spelling) we can use to print
>> the attribute in full fidelity. We can pass the spelling string to the
>> Attr, besides the syntax used as this patch did, so the printer will know
>> which syntax and spelling to select. It would be more elegant to encode
>> both information as a single index into the spelling list but I haven't
>> figured out how to do that. Any suggestions on this?
>>
>
> You would need to teach TableGen to emit a function which maps the
> attribute syntax, scope, and name to a spelling index (and to use the
> spelling index when printing the attribute). You can then use that to add a
> method to AttributeList to get the spelling index for an attribute.
>
>
>> Cheers
>> Michael
>>
>>
>> On Fri, Jan 11, 2013 at 5:14 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>>
>>> Hi,
>>>
>>> On Fri, Jan 11, 2013 at 3:53 PM, Michael Han <fragmentshaders at gmail.com>wrote:
>>>
>>>> Hi,
>>>>
>>>> Attached patch is to fix PR14922. Currently when print an attribute the
>>>> GNU syntax will always be used, even if the attribute has no GNU syntax.The
>>>> fix is to pass the syntax flag when constructing the Attr node and take
>>>> that into consideration when printing the attribute. The name of actual
>>>> attribute gets printed is read from table gen definition file so there is
>>>> still some limitations, for example, when an attribute has multiple
>>>> spellings, the first spelling is used; and the namespace of the attribute
>>>> (in case it's a C++11 attribute) is not printed. I test the patch locally
>>>> in my project which has access to Clang AST but I am not sure how to write
>>>> a stand alone test to test the attribute pretty print.  After this patch
>>>> gets in I'll send another patch which updates the SemaDeclAttr to pass the
>>>> actual syntax flag from AttributeList to Attr.
>>>
>>>
>>> I don't think this is the best approach: it still always uses the first
>>> spelling, so it still won't produce the right string for
>>> __attribute__((aligned(...))) versus __declspec(alignment(...)) versus
>>> [[gnu::aligned(...)]] versus alignas(...).
>>>
>>> Since we already have a list of possible spellings for an attribute in
>>> the attribute definition (which incorporates the syntax used), how about
>>> just storing an index into that list on the Attr?
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130116/45ea7908/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: attr-print.patch
Type: application/octet-stream
Size: 13777 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130116/45ea7908/attachment.obj>