r214407 - Automate attribute argument count semantic checking when there are variadic or optional arguments present. With this, the only time you should have to manually check attribute argument counts is when HasCustomParsing is set to true, or when you have variadic arguments that aren't really variadic (like ownership_holds and friends).

Aaron Ballman aaron at aaronballman.com
Thu Jul 31 09:37:04 PDT 2014


Author: aaronballman
Date: Thu Jul 31 11:37:04 2014
New Revision: 214407

URL: http://llvm.org/viewvc/llvm-project?rev=214407&view=rev
Log:
Automate attribute argument count semantic checking when there are variadic or optional arguments present. With this, the only time you should have to manually check attribute argument counts is when HasCustomParsing is set to true, or when you have variadic arguments that aren't really variadic (like ownership_holds and friends).

Updating the diagnostics in the launch_bounds test since they have been improved in that case. Adding a test for nonnull since it has little test coverage, but has truly variadic arguments.

Added:
    cfe/trunk/test/Sema/attr-nonnull.c
Modified:
    cfe/trunk/include/clang/Sema/AttributeList.h
    cfe/trunk/lib/Sema/AttributeList.cpp
    cfe/trunk/lib/Sema/SemaDeclAttr.cpp
    cfe/trunk/test/SemaCUDA/launch_bounds.cu
    cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Modified: cfe/trunk/include/clang/Sema/AttributeList.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=214407&r1=214406&r2=214407&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/AttributeList.h (original)
+++ cfe/trunk/include/clang/Sema/AttributeList.h Thu Jul 31 11:37:04 2014
@@ -455,6 +455,7 @@ public:
   bool hasCustomParsing() const;
   unsigned getMinArgs() const;
   unsigned getMaxArgs() const;
+  bool hasVariadicArg() const;
   bool diagnoseAppertainsTo(class Sema &S, const Decl *D) const;
   bool diagnoseLangOpts(class Sema &S) const;
   bool existsInTarget(const llvm::Triple &T) const;

Modified: cfe/trunk/lib/Sema/AttributeList.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AttributeList.cpp?rev=214407&r1=214406&r2=214407&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AttributeList.cpp (original)
+++ cfe/trunk/lib/Sema/AttributeList.cpp Thu Jul 31 11:37:04 2014
@@ -206,3 +206,11 @@ bool AttributeList::isKnownToGCC() const
 unsigned AttributeList::getSemanticSpelling() const {
   return getInfo(*this).SpellingIndexToSemanticSpelling(*this);
 }
+
+bool AttributeList::hasVariadicArg() const {
+  // If the attribute has the maximum number of optional arguments, we will
+  // claim that as being variadic. If we someday get an attribute that
+  // legitimately bumps up against that maximum, we can use another bit to track
+  // whether it's truly variadic or not.
+  return getInfo(*this).OptArgs == 15;
+}

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=214407&r1=214406&r2=214407&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Jul 31 11:37:04 2014
@@ -148,30 +148,43 @@ static unsigned getNumAttributeArgs(cons
   return Attr.getNumArgs() + Attr.hasParsedType();
 }
 
-/// \brief Check if the attribute has exactly as many args as Num. May
-/// output an error.
-static bool checkAttributeNumArgs(Sema &S, const AttributeList &Attr,
-                                  unsigned Num) {
-  if (getNumAttributeArgs(Attr) != Num) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments)
-      << Attr.getName() << Num;
+template <typename Compare>
+static bool checkAttributeNumArgsImpl(Sema &S, const AttributeList &Attr,
+                                      unsigned Num, unsigned Diag,
+                                      Compare Comp) {
+  if (Comp(getNumAttributeArgs(Attr), Num)) {
+    S.Diag(Attr.getLoc(), Diag) << Attr.getName() << Num;
     return false;
   }
 
   return true;
 }
 
+/// \brief Check if the attribute has exactly as many args as Num. May
+/// output an error.
+static bool checkAttributeNumArgs(Sema &S, const AttributeList &Attr,
+                                  unsigned Num) {
+  return checkAttributeNumArgsImpl(S, Attr, Num,
+                                   diag::err_attribute_wrong_number_arguments,
+                                   std::not_equal_to<unsigned>());
+}
+
 /// \brief Check if the attribute has at least as many args as Num. May
 /// output an error.
 static bool checkAttributeAtLeastNumArgs(Sema &S, const AttributeList &Attr,
                                          unsigned Num) {
-  if (getNumAttributeArgs(Attr) < Num) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_too_few_arguments)
-      << Attr.getName() << Num;
-    return false;
-  }
+  return checkAttributeNumArgsImpl(S, Attr, Num,
+                                   diag::err_attribute_too_few_arguments,
+                                   std::less<unsigned>());
+}
 
-  return true;
+/// \brief Check if the attribute has at most as many args as Num. May
+/// output an error.
+static bool checkAttributeAtMostNumArgs(Sema &S, const AttributeList &Attr,
+                                         unsigned Num) {
+  return checkAttributeNumArgsImpl(S, Attr, Num,
+                                   diag::err_attribute_too_many_arguments,
+                                   std::greater<unsigned>());
 }
 
 /// \brief If Expr is a valid integer constant, get the value of the integer
@@ -856,8 +869,6 @@ static void handleCallableWhenAttr(Sema
 
 static void handleParamTypestateAttr(Sema &S, Decl *D,
                                     const AttributeList &Attr) {
-  if (!checkAttributeNumArgs(S, Attr, 1)) return;
-    
   ParamTypestateAttr::ConsumedState ParamState;
   
   if (Attr.isArgIdent(0)) {
@@ -896,8 +907,6 @@ static void handleParamTypestateAttr(Sem
 
 static void handleReturnTypestateAttr(Sema &S, Decl *D,
                                       const AttributeList &Attr) {
-  if (!checkAttributeNumArgs(S, Attr, 1)) return;
-  
   ReturnTypestateAttr::ConsumedState ReturnState;
   
   if (Attr.isArgIdent(0)) {
@@ -946,9 +955,6 @@ static void handleReturnTypestateAttr(Se
 
 
 static void handleSetTypestateAttr(Sema &S, Decl *D, const AttributeList &Attr) {
-  if (!checkAttributeNumArgs(S, Attr, 1))
-    return;
-  
   if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
     return;
   
@@ -974,9 +980,6 @@ static void handleSetTypestateAttr(Sema
 
 static void handleTestTypestateAttr(Sema &S, Decl *D,
                                     const AttributeList &Attr) {
-  if (!checkAttributeNumArgs(S, Attr, 1))
-    return;
-  
   if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
     return;
   
@@ -1593,15 +1596,8 @@ static void handleUsedAttr(Sema &S, Decl
 }
 
 static void handleConstructorAttr(Sema &S, Decl *D, const AttributeList &Attr) {
-  // check the attribute arguments.
-  if (Attr.getNumArgs() > 1) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_too_many_arguments)
-      << Attr.getName() << 1;
-    return;
-  }
-
   uint32_t priority = ConstructorAttr::DefaultPriority;
-  if (Attr.getNumArgs() > 0 &&
+  if (Attr.getNumArgs() &&
       !checkUInt32Argument(S, Attr, Attr.getArgAsExpr(0), priority))
     return;
 
@@ -1611,15 +1607,8 @@ static void handleConstructorAttr(Sema &
 }
 
 static void handleDestructorAttr(Sema &S, Decl *D, const AttributeList &Attr) {
-  // check the attribute arguments.
-  if (Attr.getNumArgs() > 1) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_too_many_arguments)
-      << Attr.getName() << 1;
-    return;
-  }
-
   uint32_t priority = DestructorAttr::DefaultPriority;
-  if (Attr.getNumArgs() > 0 &&
+  if (Attr.getNumArgs() &&
       !checkUInt32Argument(S, Attr, Attr.getArgAsExpr(0), priority))
     return;
 
@@ -1631,16 +1620,9 @@ static void handleDestructorAttr(Sema &S
 template <typename AttrTy>
 static void handleAttrWithMessage(Sema &S, Decl *D,
                                   const AttributeList &Attr) {
-  unsigned NumArgs = Attr.getNumArgs();
-  if (NumArgs > 1) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_too_many_arguments)
-      << Attr.getName() << 1;
-    return;
-  }
-
   // Handle the case where the attribute has a text message.
   StringRef Str;
-  if (NumArgs == 1 && !S.checkStringLiteralArgumentAttr(Attr, 0, Str))
+  if (Attr.getNumArgs() == 1 && !S.checkStringLiteralArgumentAttr(Attr, 0, Str))
     return;
 
   D->addAttr(::new (S.Context) AttrTy(Attr.getRange(), S.Context, Str,
@@ -2043,13 +2025,6 @@ static void handleBlocksAttr(Sema &S, De
 }
 
 static void handleSentinelAttr(Sema &S, Decl *D, const AttributeList &Attr) {
-  // check the attribute arguments.
-  if (Attr.getNumArgs() > 2) {
-    S.Diag(Attr.getLoc(), diag::err_attribute_too_many_arguments)
-      << Attr.getName() << 2;
-    return;
-  }
-
   unsigned sentinel = (unsigned)SentinelAttr::DefaultSentinel;
   if (Attr.getNumArgs() > 0) {
     Expr *E = Attr.getArgAsExpr(0);
@@ -3249,14 +3224,6 @@ bool Sema::CheckRegparmAttr(const Attrib
 
 static void handleLaunchBoundsAttr(Sema &S, Decl *D,
                                    const AttributeList &Attr) {
-  // check the attribute arguments.
-  if (Attr.getNumArgs() != 1 && Attr.getNumArgs() != 2) {
-    // FIXME: 0 is not okay.
-    S.Diag(Attr.getLoc(), diag::err_attribute_too_many_arguments)
-      << Attr.getName() << 2;
-    return;
-  }
-
   uint32_t MaxThreads, MinBlocks = 0;
   if (!checkUInt32Argument(S, Attr, Attr.getArgAsExpr(0), MaxThreads, 1))
     return;
@@ -4027,11 +3994,20 @@ static bool handleCommonAttributeFeature
   if (!Attr.diagnoseLangOpts(S))
     return true;
 
-  // If there are no optional arguments, then checking for the argument count
-  // is trivial.
-  if (Attr.getMinArgs() == Attr.getMaxArgs() &&
-      !checkAttributeNumArgs(S, Attr, Attr.getMinArgs()))
-    return true;
+  if (Attr.getMinArgs() == Attr.getMaxArgs()) {
+    // If there are no optional arguments, then checking for the argument count
+    // is trivial.
+    if (!checkAttributeNumArgs(S, Attr, Attr.getMinArgs()))
+      return true;
+  } else {
+    // There are optional arguments, so checking is slightly more involved.
+    if (Attr.getMinArgs() &&
+        !checkAttributeAtLeastNumArgs(S, Attr, Attr.getMinArgs()))
+      return true;
+    else if (!Attr.hasVariadicArg() && Attr.getMaxArgs() &&
+             !checkAttributeAtMostNumArgs(S, Attr, Attr.getMaxArgs()))
+      return true;
+  }
 
   // Check whether the attribute appertains to the given subject.
   if (!Attr.diagnoseAppertainsTo(S, D))

Added: cfe/trunk/test/Sema/attr-nonnull.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-nonnull.c?rev=214407&view=auto
==============================================================================
--- cfe/trunk/test/Sema/attr-nonnull.c (added)
+++ cfe/trunk/test/Sema/attr-nonnull.c Thu Jul 31 11:37:04 2014
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+void f1(int *a1, int *a2, int *a3, int *a4, int *a5, int *a6, int *a7,
+        int *a8, int *a9, int *a10, int *a11, int *a12, int *a13, int *a14,
+        int *a15, int *a16) __attribute__((nonnull(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16)));
+
+void f2(void) __attribute__((nonnull())); // expected-warning {{'nonnull' attribute applied to function with no pointer arguments}}

Modified: cfe/trunk/test/SemaCUDA/launch_bounds.cu
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/launch_bounds.cu?rev=214407&r1=214406&r2=214407&view=diff
==============================================================================
--- cfe/trunk/test/SemaCUDA/launch_bounds.cu (original)
+++ cfe/trunk/test/SemaCUDA/launch_bounds.cu Thu Jul 31 11:37:04 2014
@@ -6,9 +6,6 @@ __launch_bounds__(128, 7) void Test1(voi
 __launch_bounds__(128) void Test2(void);
 
 __launch_bounds__(1, 2, 3) void Test3(void); // expected-error {{'launch_bounds' attribute takes no more than 2 arguments}}
-
-// FIXME: the error should read that the attribute takes exactly one or two arguments, but there
-// is no support for such a diagnostic currently.
-__launch_bounds__() void Test4(void); // expected-error {{'launch_bounds' attribute takes no more than 2 arguments}}
+__launch_bounds__() void Test4(void); // expected-error {{'launch_bounds' attribute takes at least 1 argument}}
 
 int Test5 __launch_bounds__(128, 7); // expected-warning {{'launch_bounds' attribute only applies to functions and methods}}

Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=214407&r1=214406&r2=214407&view=diff
==============================================================================
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Thu Jul 31 11:37:04 2014
@@ -206,6 +206,7 @@ namespace {
 
     virtual bool isEnumArg() const { return false; }
     virtual bool isVariadicEnumArg() const { return false; }
+    virtual bool isVariadic() const { return false; }
 
     virtual void writeImplicitCtorArgs(raw_ostream &OS) const {
       OS << getUpperName();
@@ -514,6 +515,7 @@ namespace {
           ArgSizeName(ArgName + "Size"), RangeName(getLowerName()) {}
 
     std::string getType() const { return Type; }
+    bool isVariadic() const override { return true; }
 
     void writeAccessors(raw_ostream &OS) const override {
       std::string IteratorType = getLowerName().str() + "_iterator";
@@ -2033,16 +2035,26 @@ void EmitClangAttrParsedAttrList(RecordK
   }
 }
 
+static bool isArgVariadic(const Record &R, StringRef AttrName) {
+  return createArgument(R, AttrName)->isVariadic();
+}
+
 static void emitArgInfo(const Record &R, std::stringstream &OS) {
   // This function will count the number of arguments specified for the
   // attribute and emit the number of required arguments followed by the
   // number of optional arguments.
   std::vector<Record *> Args = R.getValueAsListOfDefs("Args");
   unsigned ArgCount = 0, OptCount = 0;
+  bool HasVariadic = false;
   for (const auto *Arg : Args) {
     Arg->getValueAsBit("Optional") ? ++OptCount : ++ArgCount;
+    if (!HasVariadic && isArgVariadic(*Arg, R.getName()))
+      HasVariadic = true;
   }
-  OS << ArgCount << ", " << OptCount;
+
+  // If there is a variadic argument, we will set the optional argument count
+  // to its largest value. Since it's currently a 4-bit number, we set it to 15.
+  OS << ArgCount << ", " << (HasVariadic ? 15 : OptCount);
 }
 
 static void GenerateDefaultAppertainsTo(raw_ostream &OS) {





More information about the cfe-commits mailing list