From: Aaron Ballman Date: Mon, 16 Sep 2013 18:11:41 +0000 (+0000) Subject: Updated the way the ownership attributes are semantically diagnosed. Added test... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e1668a3a07a048eb5520410119b8b06787bfcc10;p=clang Updated the way the ownership attributes are semantically diagnosed. Added test cases for the semantics checks. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@190802 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td index 19d6848943..161e9c87d8 100644 --- a/include/clang/Basic/Attr.td +++ b/include/clang/Basic/Attr.td @@ -601,6 +601,7 @@ def Ownership : InheritableAttr { ["ownership_holds", "ownership_returns", "ownership_takes"], ["Holds", "Returns", "Takes"]>, StringArgument<"Module">, VariadicUnsignedArgument<"Args">]; + let HasCustomParsing = 1; } def Packed : InheritableAttr { diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index fd82f161e6..518190c2a4 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1829,7 +1829,7 @@ def err_attribute_no_member_pointers : Error< def err_attribute_invalid_implicit_this_argument : Error< "'%0' attribute is invalid for the implicit this argument">; def err_ownership_type : Error< - "%0 attribute only applies to %1 arguments">; + "%0 attribute only applies to %select{pointer|integer}1 arguments">; def err_format_strftime_third_parameter : Error< "strftime format attribute requires 3rd parameter to be 0">; def err_format_attribute_requires_variadic : Error< diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index 9d2b269d30..756d56ad16 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -1387,45 +1387,52 @@ static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &Attr) { Attr.getAttributeSpellingListIndex())); } +static const char *ownershipKindToDiagName(OwnershipAttr::OwnershipKind K) { + switch (K) { + case OwnershipAttr::Holds: return "'ownership_holds'"; + case OwnershipAttr::Takes: return "'ownership_takes'"; + case OwnershipAttr::Returns: return "'ownership_returns'"; + } + llvm_unreachable("unknown ownership"); +} + static void handleOwnershipAttr(Sema &S, Decl *D, const AttributeList &AL) { - // This attribute must be applied to a function declaration. - // The first argument to the attribute must be a string, - // the name of the resource, for example "malloc". - // The following arguments must be argument indexes, the arguments must be - // of integer type for Returns, otherwise of pointer type. + // This attribute must be applied to a function declaration. The first + // argument to the attribute must be an identifier, the name of the resource, + // for example: malloc. The following arguments must be argument indexes, the + // arguments must be of integer type for Returns, otherwise of pointer type. // The difference between Holds and Takes is that a pointer may still be used - // after being held. free() should be __attribute((ownership_takes)), whereas + // after being held. free() should be __attribute((ownership_takes)), whereas // a list append function may well be __attribute((ownership_holds)). if (!AL.isArgIdent(0)) { S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type) - << AL.getName()->getName() << 1 << AANT_ArgumentString; + << AL.getName() << 1 << AANT_ArgumentIdentifier; return; } + // Figure out our Kind, and check arguments while we're at it. OwnershipAttr::OwnershipKind K; switch (AL.getKind()) { case AttributeList::AT_ownership_takes: K = OwnershipAttr::Takes; if (AL.getNumArgs() < 2) { - S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) - << AL.getName() << 2; + S.Diag(AL.getLoc(), diag::err_attribute_too_few_arguments) << 2; return; } break; case AttributeList::AT_ownership_holds: K = OwnershipAttr::Holds; if (AL.getNumArgs() < 2) { - S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) - << AL.getName() << 2; + S.Diag(AL.getLoc(), diag::err_attribute_too_few_arguments) << 2; return; } break; case AttributeList::AT_ownership_returns: K = OwnershipAttr::Returns; + if (AL.getNumArgs() > 2) { - S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) - << AL.getName() << AL.getNumArgs() + 2; + S.Diag(AL.getLoc(), diag::err_attribute_too_many_arguments) << 1; return; } break; @@ -1446,7 +1453,6 @@ static void handleOwnershipAttr(Sema &S, Decl *D, const AttributeList &AL) { if (Module.startswith("__") && Module.endswith("__")) Module = Module.substr(2, Module.size() - 4); - SmallVector OwnershipArgs; for (unsigned i = 1; i < AL.getNumArgs(); ++i) { Expr *Ex = AL.getArgAsExpr(i); @@ -1455,51 +1461,35 @@ static void handleOwnershipAttr(Sema &S, Decl *D, const AttributeList &AL) { AL.getLoc(), i, Ex, Idx)) return; + // Is the function argument a pointer type? + QualType T = getFunctionOrMethodArgType(D, Idx); + int Err = -1; // No error switch (K) { - case OwnershipAttr::Takes: - case OwnershipAttr::Holds: { - // Is the function argument a pointer type? - QualType T = getFunctionOrMethodArgType(D, Idx); - if (!T->isAnyPointerType() && !T->isBlockPointerType()) { - // FIXME: Should also highlight argument in decl. - S.Diag(AL.getLoc(), diag::err_ownership_type) - << ((K==OwnershipAttr::Takes)?"ownership_takes":"ownership_holds") - << "pointer" - << Ex->getSourceRange(); - continue; - } - break; + case OwnershipAttr::Takes: + case OwnershipAttr::Holds: + if (!T->isAnyPointerType() && !T->isBlockPointerType()) + Err = 0; + break; + case OwnershipAttr::Returns: + if (!T->isIntegerType()) + Err = 1; + break; } - case OwnershipAttr::Returns: { - if (AL.getNumArgs() > 2) { - // Is the function argument an integer type? - Expr *IdxExpr = AL.getArgAsExpr(1); - llvm::APSInt ArgNum(32); - if (IdxExpr->isTypeDependent() || IdxExpr->isValueDependent() - || !IdxExpr->isIntegerConstantExpr(ArgNum, S.Context)) { - S.Diag(AL.getLoc(), diag::err_ownership_type) - << "ownership_returns" << "integer" - << IdxExpr->getSourceRange(); - return; - } - } - break; + if (-1 != Err) { + S.Diag(AL.getLoc(), diag::err_ownership_type) << AL.getName() << Err + << Ex->getSourceRange(); + return; } - } // switch // Check we don't have a conflict with another ownership attribute. for (specific_attr_iterator - i = D->specific_attr_begin(), - e = D->specific_attr_end(); - i != e; ++i) { - if ((*i)->getOwnKind() != K) { - for (const unsigned *I = (*i)->args_begin(), *E = (*i)->args_end(); - I!=E; ++I) { - if (Idx == *I) { - S.Diag(AL.getLoc(), diag::err_attributes_are_not_compatible) - << AL.getName()->getName() << "ownership_*"; - } - } + i = D->specific_attr_begin(), + e = D->specific_attr_end(); i != e; ++i) { + if ((*i)->getOwnKind() != K && (*i)->args_end() != + std::find((*i)->args_begin(), (*i)->args_end(), Idx)) { + S.Diag(AL.getLoc(), diag::err_attributes_are_not_compatible) + << AL.getName() << ownershipKindToDiagName((*i)->getOwnKind()); + return; } } OwnershipArgs.push_back(Idx); @@ -1509,12 +1499,6 @@ static void handleOwnershipAttr(Sema &S, Decl *D, const AttributeList &AL) { unsigned size = OwnershipArgs.size(); llvm::array_pod_sort(start, start + size); - if (K != OwnershipAttr::Returns && OwnershipArgs.empty()) { - S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) - << AL.getName() << 2; - return; - } - D->addAttr(::new (S.Context) OwnershipAttr(AL.getLoc(), S.Context, K, Module, start, size, AL.getAttributeSpellingListIndex())); diff --git a/test/Sema/attr-ownership.c b/test/Sema/attr-ownership.c new file mode 100644 index 0000000000..1fd97e6e5e --- /dev/null +++ b/test/Sema/attr-ownership.c @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 %s -verify + +void f1(void) __attribute__((ownership_takes("foo"))); // expected-error {{'ownership_takes' attribute requires parameter 1 to be an identifier}} +void *f2(void) __attribute__((ownership_returns(foo, 1, 2))); // expected-error {{attribute takes no more than 1 argument}} +void f3(void) __attribute__((ownership_holds(foo, 1))); // expected-error {{'ownership_holds' attribute parameter 1 is out of bounds}} +void *f4(void) __attribute__((ownership_returns(foo))); +void f5(void) __attribute__((ownership_holds(foo))); // expected-error {{attribute takes at least 2 arguments}} +void f6(void) __attribute__((ownership_holds(foo, 1, 2, 3))); // expected-error {{'ownership_holds' attribute parameter 1 is out of bounds}} +void f7(void) __attribute__((ownership_takes(foo))); // expected-error {{attribute takes at least 2 arguments}} +void f8(int *i, int *j, int k) __attribute__((ownership_holds(foo, 1, 2, 4))); // expected-error {{'ownership_holds' attribute parameter 3 is out of bounds}} + +int f9 __attribute__((ownership_takes(foo, 1))); // expected-warning {{'ownership_takes' attribute only applies to functions}} + +void f10(int i) __attribute__((ownership_holds(foo, 1))); // expected-error {{'ownership_holds' attribute only applies to pointer arguments}} +void *f11(float i) __attribute__((ownership_returns(foo, 1))); // expected-error {{'ownership_returns' attribute only applies to integer arguments}} +void *f12(float i, int k, int f, int *j) __attribute__((ownership_returns(foo, 4))); // expected-error {{'ownership_returns' attribute only applies to integer arguments}} + +void f13(int *i, int *j) __attribute__((ownership_holds(foo, 1))) __attribute__((ownership_takes(foo, 2))); +void f14(int i, int j, int *k) __attribute__((ownership_holds(foo, 3))) __attribute__((ownership_takes(foo, 3))); // expected-error {{'ownership_holds' and 'ownership_takes' attributes are not compatible}}