Here is an updated version of the second patch, incorporating feedback from code review.
Cheers, Caitlin On Wed, Jun 29, 2011 at 10:36 AM, Caitlin Sadowski <[email protected]> wrote: > Actually, I meant to send out this version of the second patch > (identical except that there is unsigned int instead of int in the > helper function). > > Cheers, > > Caitlin > > On Tue, Jun 28, 2011 at 3:23 PM, Caitlin Sadowski <[email protected]> wrote: >> Here is a second refactoring patch, building on the 1 character patch >> sent out previously, which refactors SemaDeclAttr to use a helper >> method when checking if the number of args matches, if the expected >> number is the same as the check. >> >> Also, feel free to give code review comments: >> >> http://codereview.appspot.com/4635075 >> >> Cheers, >> >> Caitlin >> >> On Mon, Jun 27, 2011 at 6:44 PM, Caitlin Sadowski <[email protected]> >> wrote: >>> This bugfix corrects a suspected mismatch between the number of >>> arguments expected and the number listed in the error message if less >>> arguments are presented in HandleReqdWorkGroupSize. >>> >>> Cheers, >>> >>> Caitlin >>> >> >
From 3413a3026c1c5600881027b97f831633c9a94ad7 Mon Sep 17 00:00:00 2001 From: Caitlin Sadowski <[email protected]> Date: Mon, 27 Jun 2011 17:49:36 -0700 Subject: [PATCH] Refactoring SemaDeclAttr to use a helper method when checking if the number of args matches, if the expected number is the same as the check. --- lib/Sema/SemaDeclAttr.cpp | 127 +++++++++++++++++---------------------------- 1 files changed, 48 insertions(+), 79 deletions(-) diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index 9d37b4c..1d6f66e 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -194,6 +194,15 @@ static inline bool isCFStringType(QualType T, ASTContext &Ctx) { return RD->getIdentifier() == &Ctx.Idents.get("__CFString"); } +static bool checkNumArgs(Sema &S, const AttributeList &Attr, unsigned int num) { + if (Attr.getNumArgs() != num) { + S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << num; + return false; + } + + return true; +} + //===----------------------------------------------------------------------===// // Attribute Implementations //===----------------------------------------------------------------------===// @@ -227,10 +236,9 @@ static void HandleExtVectorTypeAttr(Scope *scope, Decl *d, sizeExpr = Size.get(); } else { // check the attribute arguments. - if (Attr.getNumArgs() != 1) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 1; + if (!checkNumArgs(S, Attr, 1)) return; - } + sizeExpr = Attr.getArg(0); } @@ -248,10 +256,8 @@ static void HandleExtVectorTypeAttr(Scope *scope, Decl *d, static void HandlePackedAttr(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; + if (!checkNumArgs(S, Attr, 0)) return; - } if (TagDecl *TD = dyn_cast<TagDecl>(d)) TD->addAttr(::new (S.Context) PackedAttr(Attr.getLoc(), S.Context)); @@ -277,10 +283,8 @@ static void HandleMsStructAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleIBAction(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; + if (!checkNumArgs(S, Attr, 0)) return; - } // The IBAction attributes only apply to instance methods. if (ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(d)) @@ -294,10 +298,8 @@ static void HandleIBAction(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleIBOutlet(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; + if (!checkNumArgs(S, Attr, 0)) return; - } // The IBOutlet attributes only apply to instance variables of // Objective-C classes. @@ -754,10 +756,8 @@ static void HandleAliasAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleNakedAttr(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; + if (!checkNumArgs(S, Attr, 0)) return; - } if (!isa<FunctionDecl>(d)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) @@ -805,10 +805,8 @@ static void HandleMallocAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleMayAliasAttr(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; + if (!checkNumArgs(S, Attr, 0)) return; - } d->addAttr(::new (S.Context) MayAliasAttr(Attr.getLoc(), S.Context)); } @@ -861,10 +859,8 @@ static void HandleAnalyzerNoReturnAttr(Decl *d, const AttributeList &Attr, // The checking path for 'noreturn' and 'analyzer_noreturn' are different // because 'analyzer_noreturn' does not impact the type. - if (Attr.getNumArgs() != 0) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0; - return; - } + if(!checkNumArgs(S, Attr, 0)) + return; if (!isFunctionOrMethod(d) && !isa<BlockDecl>(d)) { ValueDecl *VD = dyn_cast<ValueDecl>(d); @@ -1148,10 +1144,8 @@ static void HandleAvailabilityAttr(Decl *d, const AttributeList &Attr, static void HandleVisibilityAttr(Decl *d, const AttributeList &Attr, Sema &S) { // check the attribute arguments. - if (Attr.getNumArgs() != 1) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 1; + if(!checkNumArgs(S, Attr, 1)) return; - } Expr *Arg = Attr.getArg(0); Arg = Arg->IgnoreParenCasts(); @@ -1237,10 +1231,9 @@ static void HandleObjCMethodFamilyAttr(Decl *decl, const AttributeList &attr, static void HandleObjCExceptionAttr(Decl *D, const AttributeList &Attr, Sema &S) { - if (Attr.getNumArgs() != 0) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0; + if (!checkNumArgs(S, Attr, 0)) return; - } + ObjCInterfaceDecl *OCI = dyn_cast<ObjCInterfaceDecl>(D); if (OCI == 0) { @@ -1401,10 +1394,8 @@ static void HandleSentinelAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleWarnUnusedResult(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; + if (!checkNumArgs(S, Attr, 0)) return; - } if (!isFunction(D) && !isa<ObjCMethodDecl>(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) @@ -1453,10 +1444,9 @@ static void HandleWeakAttr(Decl *d, const AttributeList &attr, Sema &S) { static void HandleWeakImportAttr(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; + if (!checkNumArgs(S, Attr, 0)) return; - } + // weak_import only applies to variable & function declarations. bool isDef = false; @@ -1482,10 +1472,8 @@ static void HandleWeakImportAttr(Decl *D, const AttributeList &Attr, Sema &S) { static void HandleReqdWorkGroupSize(Decl *D, const AttributeList &Attr, Sema &S) { // Attribute has 3 arguments. - if (Attr.getNumArgs() != 3) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 3; + if (!checkNumArgs(S, Attr, 3)) return; - } unsigned WGSize[3]; for (unsigned i = 0; i < 3; ++i) { @@ -1506,10 +1494,8 @@ static void HandleReqdWorkGroupSize(Decl *D, const AttributeList &Attr, static void HandleSectionAttr(Decl *D, const AttributeList &Attr, Sema &S) { // Attribute has no arguments. - if (Attr.getNumArgs() != 1) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 1; + if (!checkNumArgs(S, Attr, 1)) return; - } // Make sure that there is a string literal as the sections's single // argument. @@ -1571,10 +1557,8 @@ static void HandleConstAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandlePureAttr(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; + if (!checkNumArgs(S, Attr, 0)) return; - } d->addAttr(::new (S.Context) PureAttr(Attr.getLoc(), S.Context)); } @@ -1642,10 +1626,9 @@ static void HandleCleanupAttr(Decl *d, const AttributeList &Attr, Sema &S) { /// Handle __attribute__((format_arg((idx)))) attribute based on /// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html static void HandleFormatArgAttr(Decl *d, const AttributeList &Attr, Sema &S) { - if (Attr.getNumArgs() != 1) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 1; + if (!checkNumArgs(S, Attr, 1)) return; - } + if (!isFunctionOrMethod(d) || !hasFunctionProto(d)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) << Attr.getName() << ExpectedFunction; @@ -1959,10 +1942,9 @@ static void HandleFormatAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleTransparentUnionAttr(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; + if (!checkNumArgs(S, Attr, 0)) return; - } + // Try to find the underlying union declaration. RecordDecl *RD = 0; @@ -2026,10 +2008,9 @@ static void HandleTransparentUnionAttr(Decl *d, const AttributeList &Attr, static void HandleAnnotateAttr(Decl *d, const AttributeList &Attr, Sema &S) { // check the attribute arguments. - if (Attr.getNumArgs() != 1) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 1; + if (!checkNumArgs(S, Attr, 1)) return; - } + Expr *ArgExpr = Attr.getArg(0); StringLiteral *SE = dyn_cast<StringLiteral>(ArgExpr); @@ -2103,10 +2084,9 @@ static void HandleModeAttr(Decl *D, const AttributeList &Attr, Sema &S) { // the width of an int or unsigned int to the specified size. // Check that there aren't any arguments - if (Attr.getNumArgs() != 0) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0; + if (!checkNumArgs(S, Attr, 0)) return; - } + IdentifierInfo *Name = Attr.getParameterName(); if (!Name) { @@ -2265,10 +2245,8 @@ static void HandleModeAttr(Decl *D, const AttributeList &Attr, Sema &S) { static void HandleNoDebugAttr(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; + if (!checkNumArgs(S, Attr, 0)) return; - } if (!isFunctionOrMethod(d)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) @@ -2281,10 +2259,9 @@ static void HandleNoDebugAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleNoInlineAttr(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; + if (!checkNumArgs(S, Attr, 0)) return; - } + if (!isa<FunctionDecl>(d)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) @@ -2298,10 +2275,9 @@ static void HandleNoInlineAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleNoInstrumentFunctionAttr(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; + if (!checkNumArgs(S, Attr, 0)) return; - } + if (!isa<FunctionDecl>(d)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) @@ -2356,10 +2332,8 @@ static void HandleDeviceAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleGlobalAttr(Decl *d, const AttributeList &Attr, Sema &S) { if (S.LangOpts.CUDA) { // check the attribute arguments. - if (Attr.getNumArgs() != 0) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0; + if (!checkNumArgs(S, Attr, 0)) return; - } if (!isa<FunctionDecl>(d)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) @@ -2391,10 +2365,9 @@ static void HandleGlobalAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleHostAttr(Decl *d, const AttributeList &Attr, Sema &S) { if (S.LangOpts.CUDA) { // check the attribute arguments. - if (Attr.getNumArgs() != 0) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0; + if (!checkNumArgs(S, Attr, 0)) return; - } + if (!isa<FunctionDecl>(d)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) @@ -2411,10 +2384,9 @@ static void HandleHostAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleSharedAttr(Decl *d, const AttributeList &Attr, Sema &S) { if (S.LangOpts.CUDA) { // check the attribute arguments. - if (Attr.getNumArgs() != 0) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0; + if (!checkNumArgs(S, Attr, 0)) return; - } + if (!isa<VarDecl>(d)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) @@ -2430,10 +2402,8 @@ static void HandleSharedAttr(Decl *d, const AttributeList &Attr, Sema &S) { static void HandleGNUInlineAttr(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; + if (!checkNumArgs(S, Attr, 0)) return; - } FunctionDecl *Fn = dyn_cast<FunctionDecl>(d); if (Fn == 0) { @@ -2856,10 +2826,9 @@ static bool isKnownDeclSpecAttr(const AttributeList &Attr) { static void HandleUuidAttr(Decl *d, const AttributeList &Attr, Sema &S) { if (S.LangOpts.Microsoft || S.LangOpts.Borland) { // check the attribute arguments. - if (Attr.getNumArgs() != 1) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 1; + if (!checkNumArgs(S, Attr, 1)) return; - } + Expr *Arg = Attr.getArg(0); StringLiteral *Str = dyn_cast<StringLiteral>(Arg); if (Str == 0 || Str->isWide()) { -- 1.7.3.1
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
