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 <[email protected]> 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@public.**gmane.org<[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<http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
