diff mbox

ObjC/ObjC++: Add support for nonnull attribute for ObjC methods

Message ID 1306511138.936614200@www2.webmail.us
State New
Headers show

Commit Message

Nicola Pero May 27, 2011, 3:45 p.m. UTC 
This patch adds support for the "nonnull" attribute for Objective-C methods (clang has it too).

The implementation follows the existing framework and is very similar to the "format" attribute one.

Testcases included.

Ok to commit ?

Thanks

PS: There is a small unsatisfactory issue with the generated warnings, which are too correct :-)
in that they say something like

  xxx.m:89:3: warning: null argument where non-null required (argument 3) [-Wnonnull]

and the "argument 3" index includes the 2 hidden Objective-C method arguments (self and _cmd).  I'd rather
it didn't, and I'd rather that the warning said "argument 1" instead.  Note that the "format" attribute,
as implemented, already has this problem, but you never notice because the warnings it generates never
mention the argument index ;-)

But, changing the warnings requires refactoring and more radical changes (including changes to c-family),
which I'd rather leave for a separate patch.  I added a FIXME in the code about the issue.

Comments

Mike Stump May 27, 2011, 7:43 p.m. UTC | #1 
On May 27, 2011, at 8:45 AM, Nicola Pero wrote:
> This patch adds support for the "nonnull" attribute for Objective-C methods (clang has it too).

> Ok to commit ?

Ok.
diff mbox

Patch

Index: objc/ChangeLog
===================================================================
--- objc/ChangeLog      (revision 174335)
+++ objc/ChangeLog      (working copy)
@@ -1,3 +1,8 @@ 
+2011-05-27  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc-act.c (objc_decl_method_attributes): Implement nonnull
+       attribute for Objective-C methods.
+
 2011-05-21  Nicola Pero  <nicola.pero@meta-innovation.com>
 
        * config-lang.in (gtfiles): Updated order of files to fix building
Index: objc/objc-act.c
===================================================================
--- objc/objc-act.c     (revision 174335)
+++ objc/objc-act.c     (working copy)
@@ -5042,6 +5042,48 @@  objc_decl_method_attributes (tree *node, tree attr
              filtered_attributes = chainon (filtered_attributes,
                                             new_attribute);
            }
+         else if (is_attribute_p ("nonnull", name))
+           {
+             /* We need to fixup all the argument indexes by adding 2
+                for the two hidden arguments of an Objective-C method
+                invocation, similat to what we do above for the
+                "format" attribute.  */
+             /* FIXME: This works great in terms of implementing the
+                functionality, but the warnings that are produced by
+                nonnull do mention the argument index (while the
+                format ones don't).  For example, you could get
+                "warning: null argument where non-null required
+                (argument 3)".  Now in that message, "argument 3"
+                includes the 2 hidden arguments; it would be much
+                more friendly to call it "argument 1", as that would
+                be consistent with __attribute__ ((nonnnull (1))).
+                To do this, we'd need to have the C family code that
+                checks the arguments know about adding/removing 2 to
+                the argument index ... or alternatively we could
+                maybe store the "printable" argument index in
+                addition to the actual argument index ?  Some
+                refactoring is needed to do this elegantly.  */
+             tree new_attribute = copy_node (attribute);
+             tree argument = TREE_VALUE (attribute);
+             while (argument != NULL_TREE)
+               {
+                 /* Get the value of the argument and add 2.  */
+                 tree number = TREE_VALUE (argument);
+                 if (number
+                     && TREE_CODE (number) == INTEGER_CST
+                     && TREE_INT_CST_HIGH (number) == 0
+                     && TREE_INT_CST_LOW (number) != 0)
+                   {
+                     TREE_VALUE (argument)
+                       = build_int_cst (integer_type_node,
+                                        TREE_INT_CST_LOW (number) + 2);
+                   }
+                 argument = TREE_CHAIN (argument);
+               }
+
+             filtered_attributes = chainon (filtered_attributes,
+                                            new_attribute);
+           }
          else
            warning (OPT_Wattributes, "%qE attribute directive ignored", name);
        }
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 174335)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@ 
+2011-05-27  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc.dg/attributes/method-nonnull-1.m: New test.
+       * obj-c++.dg/attributes/method-nonnull-1.mm: New test.  
+
 2011-05-27  Richard Guenther  <rguenther@suse.de>
 
        * gcc.c-torture/execute/920711-1.x: Add -fwrapv.
Index: testsuite/objc.dg/attributes/method-nonnull-1.m
===================================================================
--- testsuite/objc.dg/attributes/method-nonnull-1.m     (revision 0)
+++ testsuite/objc.dg/attributes/method-nonnull-1.m     (revision 0)
@@ -0,0 +1,46 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, May 2011.  */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+#include <objc/objc.h>
+#include <stdlib.h>
+
+@interface MyArray
+{
+  Class isa;
+} 
++ (void) addObject: (id)object __attribute__ ((nonnull));
+- (void) addObject: (id)object __attribute__ ((nonnull));
+
++ (void) insertObject: (id)object  atIndex: (size_t)index __attribute__ ((nonnull (1)));
+- (void) insertObject: (id)object  atIndex: (size_t)index __attribute__ ((nonnull (1)));
+
++ (void) insertObject: (id)object  atIndex: (size_t)index  andObject: (id)anotherObject  atIndex: (size_t)anotherIndex __attribute__ ((nonnull (1, 3)));
+- (void) insertObject: (id)object  atIndex: (size_t)index  andObject: (id)anotherObject  atIndex: (size_t)anotherIndex __attribute__ ((nonnull (1, 3)));
+
+/* Test the behaviour with invalid code.  */
++ (void) removeObject: (id)object __attribute__ ((nonnull (0))); /* { dg-error "out-of-range" } */
+- (void) removeObject: (id)object __attribute__ ((nonnull (0))); /* { dg-error "out-of-range" } */
+
++ (void) removeObject: (id)object __attribute__ ((nonnull (2))); /* { dg-error "out-of-range" } */
+- (void) removeObject: (id)object __attribute__ ((nonnull (2))); /* { dg-error "out-of-range" } */
+
++ (void) removeObjectAtIndex: (size_t)object __attribute__ ((nonnull (1))); /* { dg-error "non-pointer operand" } */
+- (void) removeObjectAtIndex: (size_t)object __attribute__ ((nonnull (1))); /* { dg-error "non-pointer operand" } */
+
++ (void) removeObject: (id)object __attribute__ ((nonnull (MyArray))); /* { dg-error "invalid operand" } */
+- (void) removeObject: (id)object __attribute__ ((nonnull (MyArray))); /* { dg-error "invalid operand" } */
+@end
+
+void test (MyArray *object)
+{
+  [object addObject: object];
+  [object addObject: nil]; /* { dg-warning "null argument where non-null required" } */
+
+  [object insertObject: object atIndex: 4];
+  [object insertObject: nil    atIndex: 4]; /* { dg-warning "null argument where non-null required" } */
+
+  [object insertObject: object atIndex: 2 andObject: object atIndex: 3];
+  [object insertObject: nil    atIndex: 2 andObject: object atIndex: 3]; /* { dg-warning "null argument where non-null required" } */
+  [object insertObject: object atIndex: 2 andObject: nil    atIndex: 3]; /* { dg-warning "null argument where non-null required" } */
+}
Index: testsuite/obj-c++.dg/attributes/method-nonnull-1.mm
===================================================================
--- testsuite/obj-c++.dg/attributes/method-nonnull-1.mm (revision 0)
+++ testsuite/obj-c++.dg/attributes/method-nonnull-1.mm (revision 0)
@@ -0,0 +1,46 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, May 2011.  */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+#include <objc/objc.h>
+#include <stdlib.h>
+
+@interface MyArray
+{
+  Class isa;
+} 
++ (void) addObject: (id)object __attribute__ ((nonnull));
+- (void) addObject: (id)object __attribute__ ((nonnull));
+
++ (void) insertObject: (id)object  atIndex: (size_t)index __attribute__ ((nonnull (1)));
+- (void) insertObject: (id)object  atIndex: (size_t)index __attribute__ ((nonnull (1)));
+
++ (void) insertObject: (id)object  atIndex: (size_t)index  andObject: (id)anotherObject  atIndex: (size_t)anotherIndex __attribute__ ((nonnull (1, 3)));
+- (void) insertObject: (id)object  atIndex: (size_t)index  andObject: (id)anotherObject  atIndex: (size_t)anotherIndex __attribute__ ((nonnull (1, 3)));
+
+/* Test the behaviour with invalid code.  */
++ (void) removeObject: (id)object __attribute__ ((nonnull (0))); /* { dg-error "out-of-range" } */
+- (void) removeObject: (id)object __attribute__ ((nonnull (0))); /* { dg-error "out-of-range" } */
+
++ (void) removeObject: (id)object __attribute__ ((nonnull (2))); /* { dg-error "out-of-range" } */
+- (void) removeObject: (id)object __attribute__ ((nonnull (2))); /* { dg-error "out-of-range" } */
+
++ (void) removeObjectAtIndex: (size_t)object __attribute__ ((nonnull (1))); /* { dg-error "non-pointer operand" } */
+- (void) removeObjectAtIndex: (size_t)object __attribute__ ((nonnull (1))); /* { dg-error "non-pointer operand" } */
+
++ (void) removeObject: (id)object __attribute__ ((nonnull (MyArray))); /* { dg-error "" } */
+- (void) removeObject: (id)object __attribute__ ((nonnull (MyArray))); /* { dg-error "" } */
+@end
+
+void test (MyArray *object)
+{
+  [object addObject: object];
+  [object addObject: nil]; /* { dg-warning "null argument where non-null required" } */
+
+  [object insertObject: object atIndex: 4];
+  [object insertObject: nil    atIndex: 4]; /* { dg-warning "null argument where non-null required" } */
+
+  [object insertObject: object atIndex: 2 andObject: object atIndex: 3];
+  [object insertObject: nil    atIndex: 2 andObject: object atIndex: 3]; /* { dg-warning "null argument where non-null required" } */
+  [object insertObject: object atIndex: 2 andObject: nil    atIndex: 3]; /* { dg-warning "null argument where non-null required" } */
+}