Add some basic type checking for attributes ns_returns_retained and

cf_returns_retained. Currently this attribute can now be applied to any
Objective-C method or C function that returns a pointer or Objective-C object
type.

Modify the tablegen definition of diagnostic 'warn_attribute_wrong_decl_type' to
expect that the diagnostics infrastructure will add quotes around the attribute
name when appropriate. Alonq with this change, I modified the places where this
warning is issued to passed the attribute's IdentifierInfo* instead of having a
hard-coded C constant string.

llvm-svn: 71718

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -415,7 +415,7 @@ def warn_attribute_weak_on_local : Warning<
 def warn_attribute_weak_import_invalid_on_definition : Warning<
   "'weak_import' attribute cannot be specified on a definition">;
 def warn_attribute_wrong_decl_type : Warning<
-  "'%0' attribute only applies to %select{function|union|"
+  "%0 attribute only applies to %select{function|union|"
   "variable and function|function or method|parameter|parameter or Objective-C method}1 types">;
 def warn_gnu_inline_attribute_requires_inline : Warning<
   "'gnu_inline' attribute requires function to be marked 'inline',"
@@ -491,6 +491,9 @@ def note_attribute_overloadable_prev_overload : Note<
   "previous overload of function is here">;
 def err_attribute_overloadable_no_prototype : Error<
   "'overloadable' function %0 must have a prototype">;
+def warn_ns_attribute_wrong_return_type : Warning<
+    "%0 attribute only applies to functions or methods that "
+    "return a pointer or Objective-C object">;
 
 // Function Parameter Semantic Analysis.
 def err_param_with_void_type : Error<"argument may not have 'void' type">;

clang/lib/Sema/SemaDeclAttr.cpp

@@ -295,7 +295,7 @@ static void HandleNonNullAttr(Decl *d, const AttributeList &Attr, Sema &S) {
   // prototypes, so we ignore it as well
   if (!isFunctionOrMethod(d) || !hasFunctionProto(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "nonnull" << 0 /*function*/;
+      << Attr.getName() << 0 /*function*/;
     return;
   }
   
@@ -395,7 +395,7 @@ static void HandleAlwaysInlineAttr(Decl *d, const AttributeList &Attr,
 
   if (!isa<FunctionDecl>(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-    << "always_inline" << 0 /*function*/;
+    << Attr.getName() << 0 /*function*/;
     return;
   }
   
@@ -403,7 +403,7 @@ static void HandleAlwaysInlineAttr(Decl *d, const AttributeList &Attr,
 }
 
 static bool HandleCommonNoReturnAttr(Decl *d, const AttributeList &Attr,
-                                     Sema &S, const char *attrName) {
+                                     Sema &S) {
   // check the attribute arguments.
   if (Attr.getNumArgs() != 0) {
     S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0;
@@ -414,7 +414,7 @@ static bool HandleCommonNoReturnAttr(Decl *d, const AttributeList &Attr,
     ValueDecl *VD = dyn_cast<ValueDecl>(d);
     if (VD == 0 || !VD->getType()->isBlockPointerType()) {
       S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-        << attrName << 0 /*function*/;
+        << Attr.getName() << 0 /*function*/;
       return false;
     }
   }
@@ -423,13 +423,13 @@ static bool HandleCommonNoReturnAttr(Decl *d, const AttributeList &Attr,
 }
 
 static void HandleNoReturnAttr(Decl *d, const AttributeList &Attr, Sema &S) {
-  if (HandleCommonNoReturnAttr(d, Attr, S, "noreturn"))  
+  if (HandleCommonNoReturnAttr(d, Attr, S))  
     d->addAttr(::new (S.Context) NoReturnAttr());
 }
 
 static void HandleAnalyzerNoReturnAttr(Decl *d, const AttributeList &Attr,
                                        Sema &S) {
-  if (HandleCommonNoReturnAttr(d, Attr, S, "analyzer_noreturn"))  
+  if (HandleCommonNoReturnAttr(d, Attr, S))  
     d->addAttr(::new (S.Context) AnalyzerNoReturnAttr());
 }
 
@@ -442,7 +442,7 @@ static void HandleUnusedAttr(Decl *d, const AttributeList &Attr, Sema &S) {
   
   if (!isa<VarDecl>(d) && !isFunctionOrMethod(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "unused" << 2 /*variable and function*/;
+      << Attr.getName() << 2 /*variable and function*/;
     return;
   }
   
@@ -463,7 +463,7 @@ static void HandleUsedAttr(Decl *d, const AttributeList &Attr, Sema &S) {
     }
   } else if (!isFunctionOrMethod(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "used" << 2 /*variable and function*/;
+      << Attr.getName() << 2 /*variable and function*/;
     return;
   }
   
@@ -492,7 +492,7 @@ static void HandleConstructorAttr(Decl *d, const AttributeList &Attr, Sema &S) {
   
   if (!isa<FunctionDecl>(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "constructor" << 0 /*function*/;
+      << Attr.getName() << 0 /*function*/;
     return;
   }
 
@@ -521,7 +521,7 @@ static void HandleDestructorAttr(Decl *d, const AttributeList &Attr, Sema &S) {
   
   if (!isa<FunctionDecl>(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "destructor" << 0 /*function*/;
+      << Attr.getName() << 0 /*function*/;
     return;
   }
 
@@ -722,7 +722,7 @@ static void HandleSentinelAttr(Decl *d, const AttributeList &Attr, Sema &S) {
     }    
   } else {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "sentinel" << 3 /*function or method*/;
+      << Attr.getName() << 3 /*function or method*/;
     return;
   }
   
@@ -740,7 +740,7 @@ static void HandleWarnUnusedResult(Decl *D, const AttributeList &Attr, Sema &S)
   FunctionDecl *Fn = dyn_cast<FunctionDecl>(D);
   if (!Fn) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-    << "warn_unused_result" << 0 /*function*/;
+      << Attr.getName() << 0 /*function*/;
     return;
   }
   
@@ -757,7 +757,7 @@ static void HandleWeakAttr(Decl *D, const AttributeList &Attr, Sema &S) {
   // TODO: could also be applied to methods?
   if (!isa<FunctionDecl>(D) && !isa<VarDecl>(D)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-    << "weak" << 2 /*variable and function*/;
+      << Attr.getName() << 2 /*variable and function*/;
     return;
   }
   
@@ -782,7 +782,7 @@ static void HandleWeakImportAttr(Decl *D, const AttributeList &Attr, Sema &S) {
     return;
   } else {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-    << "weak_import" << 2 /*variable and function*/;
+    << Attr.getName() << 2 /*variable and function*/;
     return;
   }
 
@@ -813,7 +813,7 @@ static void HandleDLLImportAttr(Decl *D, const AttributeList &Attr, Sema &S) {
   FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
   if (!FD) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "dllimport" << 2 /*variable and function*/;
+      << Attr.getName() << 2 /*variable and function*/;
     return;
   }
 
@@ -858,7 +858,7 @@ static void HandleDLLExportAttr(Decl *D, const AttributeList &Attr, Sema &S) {
   FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
   if (!FD) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "dllexport" << 2 /*variable and function*/;
+      << Attr.getName() << 2 /*variable and function*/;
     return;
   }
 
@@ -903,7 +903,7 @@ static void HandleStdCallAttr(Decl *d, const AttributeList &Attr, Sema &S) {
   // Attribute can be applied only to functions.
   if (!isa<FunctionDecl>(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "stdcall" << 0 /*function*/;
+      << Attr.getName() << 0 /*function*/;
     return;
   }
 
@@ -926,7 +926,7 @@ static void HandleFastCallAttr(Decl *d, const AttributeList &Attr, Sema &S) {
 
   if (!isa<FunctionDecl>(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "fastcall" << 0 /*function*/;
+      << Attr.getName() << 0 /*function*/;
     return;
   }
 
@@ -1045,7 +1045,7 @@ static void HandleFormatAttr(Decl *d, const AttributeList &Attr, Sema &S) {
 
   if (!isFunctionOrMethod(d) || !hasFunctionProto(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "format" << 0 /*function*/;
+      << Attr.getName() << 0 /*function*/;
     return;
   }
 
@@ -1190,7 +1190,7 @@ static void HandleTransparentUnionAttr(Decl *d, const AttributeList &Attr,
 
   if (!RD || !RD->isUnion()) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "transparent_union" << 1 /*union*/;
+      << Attr.getName() << 1 /*union*/;
     return;
   }
 
@@ -1464,7 +1464,7 @@ static void HandleNodebugAttr(Decl *d, const AttributeList &Attr, Sema &S) {
 
   if (!isFunctionOrMethod(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "nodebug" << 0 /*function*/;
+      << Attr.getName() << 0 /*function*/;
     return;
   }
   
@@ -1480,7 +1480,7 @@ static void HandleNoinlineAttr(Decl *d, const AttributeList &Attr, Sema &S) {
   
   if (!isa<FunctionDecl>(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-    << "noinline" << 0 /*function*/;
+    << Attr.getName() << 0 /*function*/;
     return;
   }
   
@@ -1497,7 +1497,7 @@ static void HandleGNUInlineAttr(Decl *d, const AttributeList &Attr, Sema &S) {
   FunctionDecl *Fn = dyn_cast<FunctionDecl>(d);
   if (Fn == 0) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << "gnu_inline" << 0 /*function*/;
+      << Attr.getName() << 0 /*function*/;
     return;
   }
   
@@ -1518,7 +1518,7 @@ static void HandleRegparmAttr(Decl *d, const AttributeList &Attr, Sema &S) {
 
   if (!isFunctionOrMethod(d)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-    << "regparm" << 0 /*function*/;
+    << Attr.getName() << 0 /*function*/;
     return;
   }
 
@@ -1552,21 +1552,21 @@ static void HandleRegparmAttr(Decl *d, const AttributeList &Attr, Sema &S) {
 static void HandleNSReturnsRetainedAttr(Decl *d, const AttributeList &Attr,
                                         Sema &S) {
 
-  if (!isa<ObjCMethodDecl>(d) && !isa<FunctionDecl>(d)) {
+  QualType RetTy;
-    const char *name;
   
-    switch (Attr.getKind()) {
+  if (ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(d))
-      default:
+    RetTy = MD->getResultType();
-        assert(0 && "invalid ownership attribute");
+  else if (FunctionDecl *FD = dyn_cast<FunctionDecl>(d))
-        return;
+    RetTy = FD->getResultType();
-      case AttributeList::AT_cf_returns_retained:
+  else {
-        name = "cf_returns_retained"; break;
+    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      case AttributeList::AT_ns_returns_retained:
+        << Attr.getName() << 3 /* function or method */;
-        name = "ns_returns_retained"; break;
+    return;
-    };
+  }
   
-    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+  if (!(S.Context.isObjCNSObjectType(RetTy) || RetTy->getAsPointerType())) {
-      name << 3 /* function or method */;
+    S.Diag(Attr.getLoc(), diag::warn_ns_attribute_wrong_return_type)
+      << Attr.getName();
     return;    
   }
   

clang/test/Analysis/retain-release.m

@@ -596,11 +596,17 @@ int RDar6320065_test() {
 // Tests of ownership attributes.
 //===----------------------------------------------------------------------===//
 
+typedef NSString* MyStringTy;
+
 @interface TestOwnershipAttr : NSObject
-- (NSString*) returnsAnOwnedString  __attribute__((ns_returns_retained));
+- (NSString*) returnsAnOwnedString  __attribute__((ns_returns_retained)); // no-warning
-- (NSString*) returnsAnOwnedCFString  __attribute__((cf_returns_retained));
+- (NSString*) returnsAnOwnedCFString  __attribute__((cf_returns_retained)); // no-warning
+- (MyStringTy) returnsAnOwnedTypedString __attribute__((ns_returns_retained)); // no-warning
+- (int) returnsAnOwnedInt __attribute__((ns_returns_retained)); // expected-warning{{'ns_returns_retained' attribute only applies to functions or methods that return a pointer or Objective-C object}}
 @end
 
+static int ownership_attribute_doesnt_go_here __attribute__((ns_returns_retained)); // expected-warning{{'ns_returns_retained' attribute only applies to function or method types}}
+
 void test_attr_1(TestOwnershipAttr *X) {
   NSString *str = [X returnsAnOwnedString]; // expected-warning{{leak}}
 }