From 60ec5f80d2f681874cf2199dbd38b77346f30bb4 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Thu, 31 Jul 2014 16:37:04 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@214407 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Sema/AttributeList.h | 1 + lib/Sema/AttributeList.cpp | 8 ++ lib/Sema/SemaDeclAttr.cpp | 110 +++++++++++----------------- test/Sema/attr-nonnull.c | 7 ++ test/SemaCUDA/launch_bounds.cu | 5 +- utils/TableGen/ClangAttrEmitter.cpp | 14 +++- 6 files changed, 73 insertions(+), 72 deletions(-) create mode 100644 test/Sema/attr-nonnull.c diff --git a/include/clang/Sema/AttributeList.h b/include/clang/Sema/AttributeList.h index c21c19fd55..2b5c3ce2da 100644 --- a/include/clang/Sema/AttributeList.h +++ b/include/clang/Sema/AttributeList.h @@ -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; diff --git a/lib/Sema/AttributeList.cpp b/lib/Sema/AttributeList.cpp index 476a22b628..34af6cf63c 100644 --- a/lib/Sema/AttributeList.cpp +++ b/lib/Sema/AttributeList.cpp @@ -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; +} diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index 33577fb86f..23119450f3 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -148,30 +148,43 @@ static unsigned getNumAttributeArgs(const AttributeList &Attr) { 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 +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()); +} + /// \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()); +} - 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()); } /// \brief If Expr is a valid integer constant, get the value of the integer @@ -856,8 +869,6 @@ static void handleCallableWhenAttr(Sema &S, Decl *D, 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(Sema &S, Decl *D, 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(Sema &S, Decl *D, static void handleSetTypestateAttr(Sema &S, Decl *D, const AttributeList &Attr) { - if (!checkAttributeNumArgs(S, Attr, 1)) - return; - if (!checkForConsumableClass(S, cast(D), Attr)) return; @@ -974,9 +980,6 @@ static void handleSetTypestateAttr(Sema &S, Decl *D, const AttributeList &Attr) static void handleTestTypestateAttr(Sema &S, Decl *D, const AttributeList &Attr) { - if (!checkAttributeNumArgs(S, Attr, 1)) - return; - if (!checkForConsumableClass(S, cast(D), Attr)) return; @@ -1593,15 +1596,8 @@ static void handleUsedAttr(Sema &S, Decl *D, const AttributeList &Attr) { } 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 &S, Decl *D, const AttributeList &Attr) { } 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, Decl *D, const AttributeList &Attr) { template 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, Decl *D, const AttributeList &Attr) { } 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 AttributeList &Attr, unsigned &numParams) { 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 handleCommonAttributeFeatures(Sema &S, Scope *scope, Decl *D, 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)) diff --git a/test/Sema/attr-nonnull.c b/test/Sema/attr-nonnull.c new file mode 100644 index 0000000000..8b8f00a206 --- /dev/null +++ b/test/Sema/attr-nonnull.c @@ -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}} diff --git a/test/SemaCUDA/launch_bounds.cu b/test/SemaCUDA/launch_bounds.cu index bed7658aed..8edc41b6ce 100644 --- a/test/SemaCUDA/launch_bounds.cu +++ b/test/SemaCUDA/launch_bounds.cu @@ -6,9 +6,6 @@ __launch_bounds__(128, 7) void Test1(void); __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}} diff --git a/utils/TableGen/ClangAttrEmitter.cpp b/utils/TableGen/ClangAttrEmitter.cpp index 1790dcbd8d..0f0f89e55c 100644 --- a/utils/TableGen/ClangAttrEmitter.cpp +++ b/utils/TableGen/ClangAttrEmitter.cpp @@ -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(RecordKeeper &Records, raw_ostream &OS) { } } +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 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) { -- 2.40.0