Enhance attribute 'nonnull' to be applicable to parameters directly (infix).

This allows the following syntax:

  void baz(__attribute__((nonnull)) const char *str);

instead of:

  void baz(const char *str) __attribute__((nonnull(1)));

This also extends to Objective-C methods.

The checking logic in Sema is not as clean as I would like.  Effectively
now we need to check both the FunctionDecl/ObjCMethodDecl and the parameters,
so the point of truth is spread in two places, but the logic isn't that
cumbersome.

Implements <rdar://problem/14691443>.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@199467 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index 6426bc1ab2fa10fbe50409636f2f5ae8f0c2a703..cb739fb99f3f5c8334514d8614db6b005aa20cd8 100644 (file)
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -679,8 +679,9 @@ def NoMips16 : InheritableAttr, TargetSpecificAttr<TargetMips> {
 
 def NonNull : InheritableAttr {
   let Spellings = [GNU<"nonnull">, CXX11<"gnu", "nonnull">];
-  let Subjects = SubjectList<[ObjCMethod, FunctionLike, HasFunctionProto],
-                             WarnDiag, "ExpectedFunctionOrMethod">;
+  let Subjects = SubjectList<[ObjCMethod, FunctionLike, HasFunctionProto,
+                              ParmVar],
+                             WarnDiag, "ExpectedFunctionMethodOrParameter">;
   let Args = [VariadicUnsignedArgument<"Args">];
   let AdditionalMembers =
 [{bool isNonNull(unsigned idx) const {

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 77807cec83fcabbbc329dcb256d268084814793b..a4217254412c703c691f3c43eb8daca6b9b21a7e 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2351,6 +2351,9 @@ def err_attr_wrong_decl : Error<
 def warn_attribute_nonnull_no_pointers : Warning<
   "'nonnull' attribute applied to function with no pointer arguments">,
   InGroup<IgnoredAttributes>;
+def warn_attribute_nonnull_parm_no_args : Warning<
+  "'nonnull' attribute when used on parameters takes no arguments">,
+  InGroup<IgnoredAttributes>;
 def warn_attribute_malloc_pointer_only : Warning<
   "'malloc' attribute only applies to functions returning a pointer type">,
   InGroup<IgnoredAttributes>;

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 714e975e42f2ab65e89cb2bf4022ec09840245d1..c3acd52a1ab98f1a6db67071c82dda65ce1a9fc1 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -736,6 +736,7 @@ static void CheckNonNullArguments(Sema &S,
                                   const NamedDecl *FDecl,
                                   const Expr * const *ExprArgs,
                                   SourceLocation CallSiteLoc) {
+  // Check the attributes attached to the method/function itself.
   for (specific_attr_iterator<NonNullAttr>
        I = FDecl->specific_attr_begin<NonNullAttr>(),
        E = FDecl->specific_attr_end<NonNullAttr>(); I != E; ++I) {
@@ -747,6 +748,21 @@ static void CheckNonNullArguments(Sema &S,
       CheckNonNullArgument(S, ExprArgs[*i], CallSiteLoc);
     }
   }
+
+  // Check the attributes on the parameters.
+  ArrayRef<ParmVarDecl*> parms;
+  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(FDecl))
+    parms = FD->parameters();
+  else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(FDecl))
+    parms = MD->parameters();
+
+  unsigned argIndex = 0;
+  for (ArrayRef<ParmVarDecl*>::iterator I = parms.begin(), E = parms.end();
+       I != E; ++I, ++argIndex) {
+    const ParmVarDecl *PVD = *I;
+    if (PVD->hasAttr<NonNullAttr>())
+      CheckNonNullArgument(S, ExprArgs[argIndex], CallSiteLoc);
+  }
 }
 
 /// Handles the checks for format strings, non-POD arguments to vararg

diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index b8116a7e8a613357351e3831293106327eca052b..06290672214400d2217d15c58243e8bca872924e 100644 (file)
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -1156,6 +1156,36 @@ static void possibleTransparentUnionPointerType(QualType &T) {
     }
 }
 
+static bool attrNonNullArgCheck(Sema &S, QualType T, const AttributeList &Attr,
+                                SourceRange R) {
+  T = T.getNonReferenceType();
+  possibleTransparentUnionPointerType(T);
+
+  if (!T->isAnyPointerType() && !T->isBlockPointerType()) {
+    S.Diag(Attr.getLoc(), diag::warn_attribute_pointers_only)
+      << Attr.getName() << R;
+    return false;
+  }
+  return true;
+}
+
+static void handleNonNullAttrParameter(Sema &S, ParmVarDecl *D,
+                                       const AttributeList &Attr) {
+  // Is the argument a pointer type?
+  if (!attrNonNullArgCheck(S, D->getType(), Attr, D->getSourceRange()))
+    return;
+
+  if (Attr.getNumArgs() > 0) {
+    S.Diag(Attr.getLoc(), diag::warn_attribute_nonnull_parm_no_args)
+      << D->getSourceRange();
+    return;
+  }
+
+  D->addAttr(::new (S.Context)
+             NonNullAttr(Attr.getRange(), S.Context, 0, 0,
+                         Attr.getAttributeSpellingListIndex()));
+}
+
 static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   SmallVector<unsigned, 8> NonNullArgs;
   for (unsigned i = 0; i < Attr.getNumArgs(); ++i) {
@@ -1165,15 +1195,10 @@ static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &Attr) {
       return;
 
     // Is the function argument a pointer type?
-    QualType T = getFunctionOrMethodArgType(D, Idx).getNonReferenceType();
-    possibleTransparentUnionPointerType(T);
-    
-    if (!T->isAnyPointerType() && !T->isBlockPointerType()) {
-      // FIXME: Should also highlight argument in decl.
-      S.Diag(Attr.getLoc(), diag::warn_attribute_pointers_only)
-        << Attr.getName() << Ex->getSourceRange();
+    // FIXME: Should also highlight argument in decl in the diagnostic.
+    if (!attrNonNullArgCheck(S, getFunctionOrMethodArgType(D, Idx),
+                             Attr, Ex->getSourceRange()))
       continue;
-    }
 
     NonNullArgs.push_back(Idx);
   }
@@ -3947,7 +3972,12 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
   case AttributeList::AT_Mode:        handleModeAttr        (S, D, Attr); break;
   case AttributeList::AT_NoCommon:
     handleSimpleAttribute<NoCommonAttr>(S, D, Attr); break;
-  case AttributeList::AT_NonNull:     handleNonNullAttr     (S, D, Attr); break;
+  case AttributeList::AT_NonNull:
+      if (ParmVarDecl *PVD = dyn_cast<ParmVarDecl>(D))
+        handleNonNullAttrParameter(S, PVD, Attr);
+      else
+        handleNonNullAttr(S, D, Attr);
+      break;
   case AttributeList::AT_Overloadable:
     handleSimpleAttribute<OverloadableAttr>(S, D, Attr); break;
   case AttributeList::AT_Ownership:   handleOwnershipAttr   (S, D, Attr); break;

diff --git a/test/Sema/nonnull.c b/test/Sema/nonnull.c
index 70d3d39f8ba5744da4563ce0c095e08860e7e76d..b134b1fc665bd35437fe5fdf341a9bc2bb8ff070 100644 (file)
--- a/test/Sema/nonnull.c
+++ b/test/Sema/nonnull.c
@@ -21,3 +21,14 @@ int main(void) {
 
 void foo(const char *str) __attribute__((nonnull("foo"))); // expected-error{{'nonnull' attribute requires parameter 1 to be an integer constant}}
 void bar(int i) __attribute__((nonnull(1))); // expected-warning {{'nonnull' attribute only applies to pointer arguments}} expected-warning {{'nonnull' attribute applied to function with no pointer arguments}}
+
+void baz(__attribute__((nonnull)) const char *str);
+void baz2(__attribute__((nonnull(1))) const char *str); // expected-warning {{'nonnull' attribute when used on parameters takes no arguments}}
+void baz3(__attribute__((nonnull)) int x); // expected-warning {{'nonnull' attribute only applies to pointer arguments}}
+
+void test_baz() {
+  baz(0); // expected-warning {{null passed to a callee which requires a non-null argument}}
+  baz2(0); // no-warning
+  baz3(0); // no-warning
+}
+

diff --git a/test/SemaObjC/nonnull.m b/test/SemaObjC/nonnull.m
index 902105b924dd8b2469705144bf5a774eb088368c..d9ecbdc37eeb3c5655aa9e7811fd5d21f81b5ced 100644 (file)
--- a/test/SemaObjC/nonnull.m
+++ b/test/SemaObjC/nonnull.m
@@ -95,3 +95,17 @@ extern void DoSomethingNotNull(void *db) __attribute__((nonnull(1)));
 }
 @end
 
+__attribute__((objc_root_class))
+  @interface TestNonNullParameters
+- (void) doNotPassNullParameterNonPointerArg:(int)__attribute__((nonnull))x; // expected-warning {{'nonnull' attribute only applies to pointer arguments}}
+- (void) doNotPassNullParameter:(id)__attribute__((nonnull))x;
+- (void) doNotPassNullParameterArgIndex:(id)__attribute__((nonnull(1)))x; // expected-warning {{'nonnull' attribute when used on parameters takes no arguments}}
+- (void) doNotPassNullOnMethod:(id)x __attribute__((nonnull(1)));
+@end
+
+void test(TestNonNullParameters *f) {
+  [f doNotPassNullParameter:0]; // expected-warning {{null passed to a callee which requires a non-null argument}}
+  [f doNotPassNullParameterArgIndex:0]; // no-warning
+  [f doNotPassNullOnMethod:0]; // expected-warning {{null passed to a callee which requires a non-null argument}}
+}
+