<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Dec 11, 2013 at 1:13 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
On 05/12/2013 20:16, Aaron Ballman wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Currently, attribute names that have surrounding double underscores<br>
will have them stripped silently when determining the attribute kind.<br>
However, this doesn't make sense for all forms of attributes. For<br>
instance: __declspec(__dllexport__) should not be a valid attribute<br>
name, nor should [[__carries_dependency__]]<br>
<br>
This patch checks the attribute form and only strips the underscores<br>
for GNU attributes. However, it does also handle C++11-style GNU<br>
attribute as well, because there are existing test cases checking for<br>
that. Do we want to have the same behavior for C++11-style clang<br>
attributes?<br>
<br>
~Aaron<br>
</blockquote>
<br></div></div>
Hi Aaron,<br>
<br>
Seems fine to do as you did and limit this to spellings of GNU attributes only as doing it for C++11 attributes was clearly wrong. We can then consider limiting it further for C++11 GNU spellings if there's a need (but my understanding is that there isn't.)<br>
</blockquote><div><br></div><div>+1.</div><div><br></div><div>GCC accepts [[gnu::__attr__]], so I think we shouldn't limit this further.</div><div><br></div><div>(FWIW, GCC also accepts [[__noreturn__]]. I'm not sure whether that's a bug, but they don't accept [[__gnu__::whatever], so I guess it probably is.)</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Small style nits below, LG otherwise..<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Index: lib/Sema/AttributeList.cpp<br>
==============================<u></u>==============================<u></u>=======<br>
--- lib/Sema/AttributeList.cpp (revision 196524)<br>
+++ lib/Sema/AttributeList.cpp (working copy)<br>
@@ -121,14 +121,17 @@<br>
Syntax SyntaxUsed) {<br>
StringRef AttrName = Name->getName();<br>
<br>
- // Normalize the attribute name, __foo__ becomes foo.<br>
- if (AttrName.size() >= 4 && AttrName.startswith("__") &&<br>
+ SmallString<64> Buf;<br>
</blockquote>
<br>
Call this something like FullName, or anything more descriptive than Buf :-)<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ if (ScopeName)<br>
+ Buf += ScopeName->getName();<br>
+<br>
+ // Normalize the attribute name, __foo__ becomes foo. This is only allowable<br>
+ // for GNU attributes.<br>
+ if ((SyntaxUsed == AS_GNU || (SyntaxUsed == AS_CXX11 && Buf.equals("gnu"))) &&<br>
</blockquote>
<br>
Breaking the checks into an IsGNU bool would improve readability.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ AttrName.size() >= 4 && AttrName.startswith("__") &&<br>
AttrName.endswith("__"))<br>
AttrName = AttrName.substr(2, AttrName.size() - 4);<br>
</blockquote>
<br>
AttrName.slice(2, AttrName.size() - 2) more readable? Your call.<br>
<br>
Alp.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- SmallString<64> Buf;<br>
- if (ScopeName)<br>
- Buf += ScopeName->getName();<br>
// Ensure that in the case of C++11 attributes, we look for '::foo' if it is<br>
// unscoped.<br>
if (ScopeName || SyntaxUsed == AS_CXX11)<br>
Index: test/Sema/<u></u>MicrosoftCompatibility.c<br>
==============================<u></u>==============================<u></u>=======<br>
--- test/Sema/<u></u>MicrosoftCompatibility.c (revision 196524)<br>
+++ test/Sema/<u></u>MicrosoftCompatibility.c (working copy)<br>
@@ -19,3 +19,5 @@<br>
struct __declspec(aligned) S2 {}; /* expected-warning {{unknown __declspec attribute 'aligned' ignored}} */<br>
<br>
struct __declspec(appdomain) S3 {}; /* expected-warning {{__declspec attribute 'appdomain' is not supported}} */<br>
+<br>
+__declspec(__noreturn__) void f7(void); /* expected-warning {{unknown __declspec attribute '__noreturn__' ignored}} */<br>
Index: test/SemaCXX/attr-cxx0x.cpp<br>
==============================<u></u>==============================<u></u>=======<br>
--- test/SemaCXX/attr-cxx0x.cpp (revision 196524)<br>
+++ test/SemaCXX/attr-cxx0x.cpp (working copy)<br>
@@ -45,3 +45,6 @@<br>
static_assert(alignof(outer<<u></u>int,char>::inner<double,short><u></u>) == alignof(int) * alignof(double), "template's alignment is wrong");<br>
<br>
static_assert(alignof(int(int)<u></u>) >= 1, "alignof(function) not positive"); // expected-error{{invalid application of 'alignof' to a function type}}<br>
+<br>
+[[__carries_dependency__]] // expected-warning{{unknown attribute '__carries_dependency__' ignored}}<br>
+void func(void);<br>
</blockquote>
<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><span class="HOEnZb"><font color="#888888"><br>
</font></span></blockquote><span class="HOEnZb"><font color="#888888">
<br>
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</font></span></blockquote></div><br></div></div>