Yes, this is pretty ugly. My preferred fix would be to modify AttributeList to allow IdentifierInfo* as well as Expr* as an argument type, and include the optional first identifier/keyword in the argument list directly, rather than treating it as a second-class citizen. This would also allow us to use the normal representation for the type_tag_for_datatype attribute.<br>
<br><div class="gmail_quote">On Sun, Jan 13, 2013 at 7:29 PM, Reed Kotler <span dir="ltr"><<a href="mailto:rkotler@mips.com" target="_blank">rkotler@mips.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Seems that uniformly through semantic attribute processing, people are checking the number of args in when they really should be calling:<br>
<br>
if (Attr.hasParameterOrArguments(<u></u>)) {<br>
<br>
An attribute with a single parameter (unless it is a number like for aligned) is treated as a parameter and not part of a list of arguments.<br>
So when there is a single parameter, #args is still 0.<br>
<br>
/// AttributeList - Represents GCC's __attribute__ declaration. There are<br>
/// 4 forms of this construct...they are:<br>
///<br>
/// 1: __attribute__(( const )). ParmName/Args/NumArgs will all be unused.<br>
/// 2: __attribute__(( mode(byte) )). ParmName used, Args/NumArgs unused.<br>
/// 3: __attribute__(( format(printf, 1, 2) )). ParmName/Args/NumArgs all used.<br>
/// 4: __attribute__(( aligned(16) )). ParmName is unused, Args/Num used.<div class="im"><br>
<br>
On 01/13/2013 06:34 PM, Reed Kotler wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
I have not isolated the problem yet but in TargetAttributes.cpp, when<br>
you check the number of attribute arguments, I always get 0.<br>
<br>
But other checks I do like checking whether an attribute is attached to<br>
a function work.<br>
<br>
Attr.getNumArgs() is always coming back as 0.<br>
Attr.getName() works fine.<br>
<br>
The following test case for x86 fails also.<br>
<br>
void __attribute__((force_align_<u></u>arg_pointer(foo))) p();<br>
<br>
It fails to notice the argument foo passed to the attribute even though<br>
it checks the number of attribute args.<br>
<br>
static void HandleX86ForceAlignArgPointerA<u></u>ttr(Decl *D,<br>
const AttributeList& Attr,<br>
Sema &S) {<br>
// Check the attribute arguments.<br>
if (Attr.getNumArgs() != 0) {<br>
S.Diag(Attr.getLoc(), diag::err_attribute_wrong_<u></u>number_arguments)<br>
<< 0;<br>
return;<br>
}<br>
<br>
On 01/12/2013 06:44 AM, Dmitri Gribenko wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
On Sat, Jan 12, 2013 at 4:32 PM, Reed Kotler<br></div><div><div class="h5">
<<a href="mailto:rkotler-8NJIiSa5LzA@public.gmane.org" target="_blank">rkotler-8NJIiSa5LzA@public.<u></u>gmane.org</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I don't have commit access to the clang list (as far as I know; just<br>
llvm).<br>
<br>
Please commit if you approve the patch.<br>
</blockquote>
Since you have commit access to llvm, you have commit access to clang,<br>
and every other repository.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This patch is the first step to adding function attributes mips16 and<br>
nomips16 to the mips<br>
target.<br>
</blockquote>
Please add<br>
let Subjects = [Function];<br>
just for documentation purposes now. Somebody will come along in<br>
future and will write the necessary TableGen magic to make<br>
lib/Sema/SemaDeclAttr.cpp autogenerated. (We hope!)<br>
<br>
+ bool ProcessDeclAttribute(Scope *scope, Decl *D, const<br>
AttributeList &Attr,<br>
+ Sema &S) const {<br>
<br>
1. scope -> Scope<br>
<br>
2. Please add checks to ensure that there are no parameters passed to<br>
the attributes.<br>
<br>
3. Please add tests for (2), like:<br>
<br>
void __attribute__((mips16(foo))) foo32(); // expected-error {{whatever}}<br>
void __attribute__((mips16(10))) foo32(); // expected-error {{whatever}}<br>
<br>
and same for nomips16.<br>
<br>
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only<br>
-verify %s<br>
<br>
Triple is not needed in a target-independent test.<br>
<br>
+int main() {<br>
+ foo32();<br>
+ foo16();<br>
+}<br>
<br>
That is not needed.<br>
<br>
Another suggestion: should we reject these attributes when we are not<br>
targeting mips?<br>
<br>
Dmitri<br>
<br>
</div></div></blockquote></blockquote><div class="HOEnZb"><div class="h5">
<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><br>
</div></div></blockquote></div><br>