[cfe-commits] patch for review

[Reed Kotler]

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
>> <rkotler-8NJIiSa5LzA at public.gmane.org> 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
>>