Seems that uniformly through semantic attribute processing, people are checking the number of args in when they really should be calling:

 if (Attr.hasParameterOrArguments()) {

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.
So when there is a single parameter, #args is still 0.

/// AttributeList - Represents GCC's __attribute__ declaration. There are
/// 4 forms of this construct...they are:
///
/// 1: __attribute__(( const )). ParmName/Args/NumArgs will all be unused.
/// 2: __attribute__(( mode(byte) )). ParmName used, Args/NumArgs unused.
/// 3: __attribute__(( format(printf, 1, 2) )). ParmName/Args/NumArgs all used.
/// 4: __attribute__(( aligned(16) )). ParmName is unused, Args/Num used.

On 01/13/2013 06:34 PM, Reed Kotler wrote:
I have not isolated the problem yet but in TargetAttributes.cpp, when
you check the number of attribute arguments, I always get 0.

But other checks I do like checking whether an attribute is attached to
a function work.

Attr.getNumArgs() is always coming back as 0.
Attr.getName() works fine.

The following test case for x86 fails also.

void  __attribute__((force_align_arg_pointer(foo))) p();

It fails to notice the argument foo passed to the attribute even though
it checks the number of attribute args.

static void HandleX86ForceAlignArgPointerAttr(Decl *D,
                                               const AttributeList& Attr,
                                               Sema &S) {
   // Check the attribute arguments.
   if (Attr.getNumArgs() != 0) {
     S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments)
<< 0;
     return;
   }

On 01/12/2013 06:44 AM, Dmitri Gribenko wrote:
On Sat, Jan 12, 2013 at 4:32 PM, Reed Kotler
<[email protected]> wrote:
I don't have commit access to the clang list (as far as I know; just
llvm).

Please commit if you approve the patch.
Since you have commit access to llvm, you have commit access to clang,
and every other repository.

This patch is the first step to adding function attributes mips16 and
nomips16 to the mips
target.
Please add
let Subjects = [Function];
just for documentation purposes now.  Somebody will come along in
future and will write the necessary TableGen magic to make
lib/Sema/SemaDeclAttr.cpp autogenerated.  (We hope!)

+    bool ProcessDeclAttribute(Scope *scope, Decl *D, const
AttributeList &Attr,
+                              Sema &S) const {

1. scope -> Scope

2. Please add checks to ensure that there are no parameters passed to
the attributes.

3. Please add tests for (2), like:

void __attribute__((mips16(foo))) foo32(); // expected-error {{whatever}}
void __attribute__((mips16(10))) foo32(); // expected-error {{whatever}}

and same for nomips16.

+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only
-verify %s

Triple is not needed in a target-independent test.

+int main() {
+  foo32();
+  foo16();
+}

That is not needed.

Another suggestion: should we reject these attributes when we are not
targeting mips?

Dmitri


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to