r195302 - Refactored integer argument checking code into a helper method. Removes a considerable amount of duplicated code.

Aaron Ballman aaron at aaronballman.com
Wed Nov 20 17:50:40 PST 2013


Author: aaronballman
Date: Wed Nov 20 19:50:40 2013
New Revision: 195302

URL: http://llvm.org/viewvc/llvm-project?rev=195302&view=rev
Log:
Refactored integer argument checking code into a helper method. Removes a considerable amount of duplicated code.

Modified:
    cfe/trunk/lib/Sema/SemaDeclAttr.cpp
    cfe/trunk/test/Sema/constructor-attribute.c

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=195302&r1=195301&r2=195302&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Nov 20 19:50:40 2013
@@ -241,6 +241,28 @@ static bool checkAttributeAtLeastNumArgs
   return true;
 }
 
+/// \brief If Expr is a valid integer constant, get the value of the integer
+/// expression and return success or failure. May output an error.
+static bool checkUInt32Argument(Sema &S, const AttributeList &Attr,
+                                const Expr *Expr, uint32_t &Val,
+                                unsigned Idx = UINT_MAX) {
+  llvm::APSInt I(32);
+  if (Expr->isTypeDependent() || Expr->isValueDependent() ||
+      !Expr->isIntegerConstantExpr(I, S.Context)) {
+    if (Idx != UINT_MAX)
+      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
+        << Attr.getName() << Idx << AANT_ArgumentIntegerConstant
+        << Expr->getSourceRange();
+    else
+      S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
+        << Attr.getName() << AANT_ArgumentIntegerConstant
+        << Expr->getSourceRange();
+    return false;
+  }
+  Val = (uint32_t)I.getZExtValue();
+  return true;
+}
+
 /// \brief Check if IdxExpr is a valid argument index for a function or
 /// instance method D.  May output an error.
 ///
@@ -2035,19 +2057,10 @@ static void handleConstructorAttr(Sema &
     return;
   }
 
-  int priority = ConstructorAttr::DefaultPriority;
-  if (Attr.getNumArgs() > 0) {
-    Expr *E = Attr.getArgAsExpr(0);
-    llvm::APSInt Idx(32);
-    if (E->isTypeDependent() || E->isValueDependent() ||
-        !E->isIntegerConstantExpr(Idx, S.Context)) {
-      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
-        << Attr.getName() << 1 << AANT_ArgumentIntegerConstant
-        << E->getSourceRange();
-      return;
-    }
-    priority = Idx.getZExtValue();
-  }
+  uint32_t priority = ConstructorAttr::DefaultPriority;
+  if (Attr.getNumArgs() > 0 &&
+      !checkUInt32Argument(S, Attr, Attr.getArgAsExpr(0), priority))
+    return;
 
   if (!isa<FunctionDecl>(D)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
@@ -2067,19 +2080,10 @@ static void handleDestructorAttr(Sema &S
     return;
   }
 
-  int priority = DestructorAttr::DefaultPriority;
-  if (Attr.getNumArgs() > 0) {
-    Expr *E = Attr.getArgAsExpr(0);
-    llvm::APSInt Idx(32);
-    if (E->isTypeDependent() || E->isValueDependent() ||
-        !E->isIntegerConstantExpr(Idx, S.Context)) {
-      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
-        << Attr.getName() << 1 << AANT_ArgumentIntegerConstant
-        << E->getSourceRange();
-      return;
-    }
-    priority = Idx.getZExtValue();
-  }
+  uint32_t priority = ConstructorAttr::DefaultPriority;
+  if (Attr.getNumArgs() > 0 &&
+      !checkUInt32Argument(S, Attr, Attr.getArgAsExpr(0), priority))
+    return;
 
   if (!isa<FunctionDecl>(D)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
@@ -2726,19 +2730,10 @@ static void handleWeakImportAttr(Sema &S
 // Handles reqd_work_group_size and work_group_size_hint.
 static void handleWorkGroupSize(Sema &S, Decl *D,
                                 const AttributeList &Attr) {
-  unsigned WGSize[3];
-  for (unsigned i = 0; i < 3; ++i) {
-    Expr *E = Attr.getArgAsExpr(i);
-    llvm::APSInt ArgNum(32);
-    if (E->isTypeDependent() || E->isValueDependent() ||
-        !E->isIntegerConstantExpr(ArgNum, S.Context)) {
-      S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
-        << Attr.getName() << AANT_ArgumentIntegerConstant
-        << E->getSourceRange();
+  uint32_t WGSize[3];
+  for (unsigned i = 0; i < 3; ++i)
+    if (!checkUInt32Argument(S, Attr, Attr.getArgAsExpr(i), WGSize[i], i))
       return;
-    }
-    WGSize[i] = (unsigned) ArgNum.getZExtValue();
-  }
 
   if (Attr.getKind() == AttributeList::AT_ReqdWorkGroupSize
     && D->hasAttr<ReqdWorkGroupSizeAttr>()) {
@@ -3043,22 +3038,17 @@ static void handleInitPriorityAttr(Sema
     Attr.setInvalid();
     return;
   }
-  
-  Expr *priorityExpr = Attr.getArgAsExpr(0);
-  
-  llvm::APSInt priority(32);
-  if (priorityExpr->isTypeDependent() || priorityExpr->isValueDependent() ||
-      !priorityExpr->isIntegerConstantExpr(priority, S.Context)) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
-      << Attr.getName() << AANT_ArgumentIntegerConstant
-      << priorityExpr->getSourceRange();
+
+  Expr *E = Attr.getArgAsExpr(0);
+  uint32_t prioritynum;
+  if (!checkUInt32Argument(S, Attr, E, prioritynum)) {
     Attr.setInvalid();
     return;
   }
-  unsigned prioritynum = priority.getZExtValue();
+
   if (prioritynum < 101 || prioritynum > 65535) {
     S.Diag(Attr.getLoc(), diag::err_attribute_argument_outof_range)
-    <<  priorityExpr->getSourceRange();
+      << E->getSourceRange();
     Attr.setInvalid();
     return;
   }
@@ -3111,7 +3101,6 @@ static void handleFormatAttr(Sema &S, De
   // counted from one.
   bool HasImplicitThisParam = isInstanceMethod(D);
   unsigned NumArgs  = getFunctionOrMethodNumArgs(D) + HasImplicitThisParam;
-  unsigned FirstIdx = 1;
 
   IdentifierInfo *II = Attr.getArgAsIdent(0)->Ident;
   StringRef Format = II->getName();
@@ -3137,23 +3126,18 @@ static void handleFormatAttr(Sema &S, De
 
   // checks for the 2nd argument
   Expr *IdxExpr = Attr.getArgAsExpr(1);
-  llvm::APSInt Idx(32);
-  if (IdxExpr->isTypeDependent() || IdxExpr->isValueDependent() ||
-      !IdxExpr->isIntegerConstantExpr(Idx, S.Context)) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
-      << Attr.getName() << 2 << AANT_ArgumentIntegerConstant
-      << IdxExpr->getSourceRange();
+  uint32_t Idx;
+  if (!checkUInt32Argument(S, Attr, IdxExpr, Idx, 2))
     return;
-  }
 
-  if (Idx.getZExtValue() < FirstIdx || Idx.getZExtValue() > NumArgs) {
+  if (Idx < 1 || Idx > NumArgs) {
     S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_bounds)
       << "format" << 2 << IdxExpr->getSourceRange();
     return;
   }
 
   // FIXME: Do we need to bounds check?
-  unsigned ArgIdx = Idx.getZExtValue() - 1;
+  unsigned ArgIdx = Idx - 1;
 
   if (HasImplicitThisParam) {
     if (ArgIdx == 0) {
@@ -3193,14 +3177,9 @@ static void handleFormatAttr(Sema &S, De
 
   // check the 3rd argument
   Expr *FirstArgExpr = Attr.getArgAsExpr(2);
-  llvm::APSInt FirstArg(32);
-  if (FirstArgExpr->isTypeDependent() || FirstArgExpr->isValueDependent() ||
-      !FirstArgExpr->isIntegerConstantExpr(FirstArg, S.Context)) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
-      << Attr.getName() << 3 << AANT_ArgumentIntegerConstant
-      << FirstArgExpr->getSourceRange();
+  uint32_t FirstArg;
+  if (!checkUInt32Argument(S, Attr, FirstArgExpr, FirstArg, 3))
     return;
-  }
 
   // check if the function is variadic if the 3rd argument non-zero
   if (FirstArg != 0) {
@@ -3228,8 +3207,7 @@ static void handleFormatAttr(Sema &S, De
   }
 
   FormatAttr *NewAttr = S.mergeFormatAttr(D, Attr.getRange(), II,
-                                          Idx.getZExtValue(),
-                                          FirstArg.getZExtValue(),
+                                          Idx, FirstArg,
                                           Attr.getAttributeSpellingListIndex());
   if (NewAttr)
     D->addAttr(NewAttr);
@@ -3853,23 +3831,19 @@ static void handleCallConvAttr(Sema &S,
   }
 }
 
-static void handleOpenCLKernelAttr(Sema &S, Decl *D, const AttributeList &Attr){
+static void handleOpenCLKernelAttr(Sema &S, Decl *D,
+                                   const AttributeList &Attr) {
   D->addAttr(::new (S.Context) OpenCLKernelAttr(Attr.getRange(), S.Context));
 }
 
-static void handleOpenCLImageAccessAttr(Sema &S, Decl *D, const AttributeList &Attr){
-  Expr *E = Attr.getArgAsExpr(0);
-  llvm::APSInt ArgNum(32);
-  if (E->isTypeDependent() || E->isValueDependent() ||
-      !E->isIntegerConstantExpr(ArgNum, S.Context)) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
-      << Attr.getName() << AANT_ArgumentIntegerConstant
-      << E->getSourceRange();
+static void handleOpenCLImageAccessAttr(Sema &S, Decl *D,
+                                        const AttributeList &Attr) {
+  uint32_t ArgNum;
+  if (!checkUInt32Argument(S, Attr, Attr.getArgAsExpr(0), ArgNum))
     return;
-  }
 
-  D->addAttr(::new (S.Context) OpenCLImageAccessAttr(
-    Attr.getRange(), S.Context, ArgNum.getZExtValue()));
+  D->addAttr(::new (S.Context) OpenCLImageAccessAttr(Attr.getRange(),
+                                                     S.Context, ArgNum));
 }
 
 bool Sema::CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC, 
@@ -3966,13 +3940,9 @@ bool Sema::CheckRegparmAttr(const Attrib
     return true;
   }
 
+  uint32_t NP;
   Expr *NumParamsExpr = Attr.getArgAsExpr(0);
-  llvm::APSInt NumParams(32);
-  if (NumParamsExpr->isTypeDependent() || NumParamsExpr->isValueDependent() ||
-      !NumParamsExpr->isIntegerConstantExpr(NumParams, Context)) {
-    Diag(Attr.getLoc(), diag::err_attribute_argument_type)
-      << Attr.getName() << AANT_ArgumentIntegerConstant
-      << NumParamsExpr->getSourceRange();
+  if (!checkUInt32Argument(*this, Attr, NumParamsExpr, NP)) {
     Attr.setInvalid();
     return true;
   }
@@ -3984,7 +3954,7 @@ bool Sema::CheckRegparmAttr(const Attrib
     return true;
   }
 
-  numParams = NumParams.getZExtValue();
+  numParams = NP;
   if (numParams > Context.getTargetInfo().getRegParmMax()) {
     Diag(Attr.getLoc(), diag::err_attribute_regparm_invalid_number)
       << Context.getTargetInfo().getRegParmMax() << NumParamsExpr->getSourceRange();
@@ -4010,34 +3980,14 @@ static void handleLaunchBoundsAttr(Sema
       return;
     }
 
-    Expr *MaxThreadsExpr = Attr.getArgAsExpr(0);
-    llvm::APSInt MaxThreads(32);
-    if (MaxThreadsExpr->isTypeDependent() ||
-        MaxThreadsExpr->isValueDependent() ||
-        !MaxThreadsExpr->isIntegerConstantExpr(MaxThreads, S.Context)) {
-      S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
-        << Attr.getName() << 1 << AANT_ArgumentIntegerConstant
-        << MaxThreadsExpr->getSourceRange();
+    uint32_t MaxThreads, MinBlocks;
+    if (!checkUInt32Argument(S, Attr, Attr.getArgAsExpr(0), MaxThreads, 1) ||
+        !checkUInt32Argument(S, Attr, Attr.getArgAsExpr(1), MinBlocks, 2))
       return;
-    }
-
-    llvm::APSInt MinBlocks(32);
-    if (Attr.getNumArgs() > 1) {
-      Expr *MinBlocksExpr = Attr.getArgAsExpr(1);
-      if (MinBlocksExpr->isTypeDependent() ||
-          MinBlocksExpr->isValueDependent() ||
-          !MinBlocksExpr->isIntegerConstantExpr(MinBlocks, S.Context)) {
-        S.Diag(Attr.getLoc(), diag::err_attribute_argument_n_type)
-          << Attr.getName() << 2 << AANT_ArgumentIntegerConstant
-          << MinBlocksExpr->getSourceRange();
-        return;
-      }
-    }
 
     D->addAttr(::new (S.Context)
                CUDALaunchBoundsAttr(Attr.getRange(), S.Context,
-                                    MaxThreads.getZExtValue(),
-                                    MinBlocks.getZExtValue(),
+                                    MaxThreads, MinBlocks,
                                     Attr.getAttributeSpellingListIndex()));
   } else {
     S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << "launch_bounds";

Modified: cfe/trunk/test/Sema/constructor-attribute.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/constructor-attribute.c?rev=195302&r1=195301&r2=195302&view=diff
==============================================================================
--- cfe/trunk/test/Sema/constructor-attribute.c (original)
+++ cfe/trunk/test/Sema/constructor-attribute.c Wed Nov 20 19:50:40 2013
@@ -4,12 +4,12 @@ int x __attribute__((constructor)); // e
 int f() __attribute__((constructor));
 int f() __attribute__((constructor(1)));
 int f() __attribute__((constructor(1,2))); // expected-error {{attribute takes no more than 1 argument}}
-int f() __attribute__((constructor(1.0))); // expected-error {{'constructor' attribute requires parameter 1 to be an integer constant}}
+int f() __attribute__((constructor(1.0))); // expected-error {{'constructor' attribute requires an integer constant}}
 
 int x __attribute__((destructor)); // expected-warning {{'destructor' attribute only applies to functions}}
 int f() __attribute__((destructor));
 int f() __attribute__((destructor(1)));
 int f() __attribute__((destructor(1,2))); // expected-error {{attribute takes no more than 1 argument}}
-int f() __attribute__((destructor(1.0))); // expected-error {{'destructor' attribute requires parameter 1 to be an integer constant}}
+int f() __attribute__((destructor(1.0))); // expected-error {{'destructor' attribute requires an integer constant}}
 
 





More information about the cfe-commits mailing list