Clang attributes issue

Project Infrastructure LLVM Dev List Archives
1

Hi All,

Sorry for my previous email: there was some issue with my email system.

Hope you don’t have any problems with reading my message now.

Aaron,

Not long ago I met a problem related to clang attributes implementation. I tried to add a new CXX11 attribute with separated namespace like here:

def FooAligned : InheritableAttr {

let Spellings = [CXX11<“_Foo_attrs”, “aligned”>];

….

But I was not able to do it because name “aligned” was used in another well-known attribute. After some research I found that the problem was in file “AttrParserStringSwitches.inc” generated by TableGen. Its content was unaware about namespaces. As result my new “aligned” attribute was mentioned like here:

#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)

….

.Case(“aligned”, true)

….

#endif // CLANG_ATTR_IDENTIFIER_ARG_LIST

and there was real ambiguity in selection of proper attribute: GNU::aligned, CXX11::aligned or CXX11:: _Foo_attrs::aligned.

The simplest way to resolve the issue is to change the new attribute name but why should I do it if I use the unique namespace?

In my case I created a special patch allowing me to generate full qualified names of attributes inside CLANG_ATTR_IDENTIFIER_ARG_LIST section:

#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)

….

.Case(“CXX11:: _Foo_attrs::aligned”, true)

….

#endif // CLANG_ATTR_IDENTIFIER_ARG_LIST

My problem was resolved with this patch.

Is it interesting for anybody?

Should I create the corresponding bug record in Bugzilla?

Do you like to review my patch?

What about other sections inside “AttrParserStringSwitches.inc” using unqualified attribute names?

Andrew V. Tischenko

2

Hi All,

Sorry for my previous email: there was some issue with my email system.

Hope you don't have any problems with reading my message now.

Aaron,

Not long ago I met a problem related to clang attributes implementation. I
tried to add a new CXX11 attribute with separated namespace like here:

def FooAligned : InheritableAttr {

  let Spellings = [CXX11<"_Foo_attrs", "aligned">];

  ….

But I was not able to do it because name “aligned” was used in another
well-known attribute. After some research I found that the problem was in
file “AttrParserStringSwitches.inc” generated by TableGen. Its content was
unaware about namespaces. As result my new “aligned” attribute was mentioned
like here:

#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)

….

.Case("aligned", true)

….

#endif // CLANG_ATTR_IDENTIFIER_ARG_LIST

and there was real ambiguity in selection of proper attribute: GNU::aligned,
CXX11::aligned or CXX11:: _Foo_attrs::aligned.

The simplest way to resolve the issue is to change the new attribute name
but why should I do it if I use the unique namespace?

Since the attribute is in its own namespace, it *really* should not
have to be unique. That's a bug.

In my case I created a special patch allowing me to generate full qualified
names of attributes inside CLANG_ATTR_IDENTIFIER_ARG_LIST section:

#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)

….

.Case("CXX11:: _Foo_attrs::aligned", true)

….

#endif // CLANG_ATTR_IDENTIFIER_ARG_LIST

My problem was resolved with this patch.

Is it interesting for anybody?

Should I create the corresponding bug record in Bugzilla?

Do you like to review my patch?

What about other sections inside “AttrParserStringSwitches.inc” using
unqualified attribute names?

I would be interested in reviewing your patch if you would like to go
through the process. If you would prefer not to, or don't have the
time, etc, I am happy to spend a bit of effort on this as well. I
don't think AttrParserStringSwitches.inc should have fully-qualified
names in the string switch cases. Instead, it should be modeled
(slightly) after AttrHasAttributeImpl.inc where it accepts the syntax
used as well as the scope and attribute name.

Thanks!

~Aaron

3

I’m not sure I got your idea. AttrHasAttributeImpl.inc works perfectly with new namespaces but it is not enough that’s why I was forced to change AttrParserStringSwitches.inc. (in fact I changed TableGen of course.)
It seems I don’t understand something that’s why it’d be better if you implement the fix by yourself.

Andrew V. Tischenko

4

This patch still requires quite a bit of cleanup before being ready
for committing, but does it solve the issue for you locally with your
custom attribute?

~Aaron

attr.patch (12.8 KB)