[cfe-commits] patch for review

[Richard Smith]

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.

On Sun, Jan 13, 2013 at 7:29 PM, Reed Kotler <rkotler at mips.com> wrote:

> 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 HandleX86ForceAlignArgPointerA**ttr(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<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
>>>
>>>
> ______________________________**_________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/**mailman/listinfo/cfe-commits<http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130113/0e0d5c51/attachment.html>