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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150810/9368ec9d/attachment.html>
On Mon, Aug 10, 2015 at 4:43 AM, Андрей Тищенко <andrew.v.tischenko at gmail.com> wrote:> 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> > > > Andrew V. Tischenko > >
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 2015-08-10 20:53 GMT+03:00 Aaron Ballman <aaron at aaronballman.com>:> On Mon, Aug 10, 2015 at 4:43 AM, Андрей Тищенко > <andrew.v.tischenko at gmail.com> wrote: > > 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 > > > > > > > > > Andrew V. Tischenko > > > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150813/cef651e3/attachment.html>