Objective-C : Implement NSObject attribute.

Message ID 761AAADA-26F5-4BCA-8E9B-8338F3F371AC@sandoe.co.uk
State New
Headers show
Series
  • Objective-C : Implement NSObject attribute.
Related show

Commit Message

Iain Sandoe Nov. 6, 2020, 8:18 p.m.
Hi

Originally, I had this as a Darwin-only patch, however GNUStep
also makes use of NSObject and similar constructs, so this really
needs to be available to linux-gnu as well.

tested across several Darwin versions and on x86_64-linux-gnu.

OK for the c-family changes?
thanks
Iain

-- 
2.24.1

Comments

Joseph Myers Nov. 6, 2020, 9:59 p.m. | #1
On Fri, 6 Nov 2020, Iain Sandoe wrote:

> Hi

> 

> Originally, I had this as a Darwin-only patch, however GNUStep

> also makes use of NSObject and similar constructs, so this really

> needs to be available to linux-gnu as well.

> 

> tested across several Darwin versions and on x86_64-linux-gnu.

> 

> OK for the c-family changes?


OK.

-- 
Joseph S. Myers
joseph@codesourcery.com
Michel Morin via Gcc-patches Nov. 10, 2020, 11:11 p.m. | #2
On 11/6/20 1:18 PM, Iain Sandoe wrote:
> Hi

>

> Originally, I had this as a Darwin-only patch, however GNUStep

> also makes use of NSObject and similar constructs, so this really

> needs to be available to linux-gnu as well.

>

> tested across several Darwin versions and on x86_64-linux-gnu.

>

> OK for the c-family changes?

> thanks

> Iain

>

> =====

>

> This attribute allows pointers to be marked as pointers to

> an NSObject-compatible object.  This allows for additional

> checking of assignment etc. when referring to pointers to

> opaque types (or type-erased via a void * cast).

>

> gcc/c-family/ChangeLog:

>

> 	* c-attribs.c (handle_nsobject_attribute): New.

> 	* c.opt: Add WNSObject-attribute.

>

> gcc/objc/ChangeLog:

>

> 	* objc-act.c (objc_compare_types): Handle NSObject type

> 	attributes.

> 	(objc_type_valid_for_messaging): Likewise.

>

> gcc/testsuite/ChangeLog:

>

> 	* obj-c++.dg/attributes/nsobject-01.mm: New test.

> 	* objc.dg/attributes/nsobject-01.m: New test.


OK with a suitable update to documentation.  I have no idea the
attribute is supposed to do.


jeff
Iain Sandoe Nov. 10, 2020, 11:43 p.m. | #3
Hi Jeff,

Jeff Law <law@redhat.com> wrote:
>>

>> This attribute allows pointers to be marked as pointers to

>> an NSObject-compatible object.  This allows for additional


>

> OK with a suitable update to documentation.  I have no idea the

> attribute is supposed to do.


Joseph ack’d this and I applied it to master ..

.. this was intentionally undocumented.

I originally wrote documentation for it.  However, it turns out that it
is undocumented in both clang (and as far as I can tell Apple’s)
material.

This is perhaps because it’s not really intended (or recommended)
for end-user code, but for “internal” cases when writing libraries etc.

WDYT? (I’m happy to ressurect the documentation I wrote and
post/apply it).

thanks
Iain
Michel Morin via Gcc-patches Nov. 10, 2020, 11:49 p.m. | #4
On 11/10/20 4:43 PM, Iain Sandoe wrote:
> Hi Jeff,

>

> Jeff Law <law@redhat.com> wrote:

>>>

>>> This attribute allows pointers to be marked as pointers to

>>> an NSObject-compatible object.  This allows for additional

>

>>

>> OK with a suitable update to documentation.  I have no idea the

>> attribute is supposed to do.

>

> Joseph ack’d this and I applied it to master ..

>

> .. this was intentionally undocumented.

>

> I originally wrote documentation for it.  However, it turns out that it

> is undocumented in both clang (and as far as I can tell Apple’s)

> material.

>

> This is perhaps because it’s not really intended (or recommended)

> for end-user code, but for “internal” cases when writing libraries etc.

>

> WDYT? (I’m happy to ressurect the documentation I wrote and

> post/apply it).


Your call -- if users aren't supposed to use the attribute, then
documentation is much less important ;-)

Jeff

Patch

=====

This attribute allows pointers to be marked as pointers to
an NSObject-compatible object.  This allows for additional
checking of assignment etc. when referring to pointers to
opaque types (or type-erased via a void * cast).

gcc/c-family/ChangeLog:

	* c-attribs.c (handle_nsobject_attribute): New.
	* c.opt: Add WNSObject-attribute.

gcc/objc/ChangeLog:

	* objc-act.c (objc_compare_types): Handle NSObject type
	attributes.
	(objc_type_valid_for_messaging): Likewise.

gcc/testsuite/ChangeLog:

	* obj-c++.dg/attributes/nsobject-01.mm: New test.
	* objc.dg/attributes/nsobject-01.m: New test.
---
 gcc/c-family/c-attribs.c                      | 39 +++++++++++
 gcc/c-family/c.opt                            |  4 ++
 gcc/objc/objc-act.c                           | 39 +++++++++--
 .../obj-c++.dg/attributes/nsobject-01.mm      | 66 +++++++++++++++++++
 .../objc.dg/attributes/nsobject-01.m          | 66 +++++++++++++++++++
 5 files changed, 207 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/obj-c++.dg/attributes/nsobject-01.mm
 create mode 100644 gcc/testsuite/objc.dg/attributes/nsobject-01.m

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 65cba5074e5..f1680820ecd 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -157,6 +157,7 @@  static tree handle_designated_init_attribute (tree *, tree, tree, int, bool *);
 static tree handle_patchable_function_entry_attribute (tree *, tree, tree,
 						       int, bool *);
 static tree handle_copy_attribute (tree *, tree, tree, int, bool *);
+static tree handle_nsobject_attribute (tree *, tree, tree, int, bool *);
 
 /* Helper to define attribute exclusions.  */
 #define ATTR_EXCL(name, function, type, variable)	\
@@ -509,6 +510,9 @@  const struct attribute_spec c_common_attribute_table[] =
 			      handle_noinit_attribute, attr_noinit_exclusions },
   { "access",		      1, 3, false, true, true, false,
 			      handle_access_attribute, NULL },
+  /* Attributes used by Objective-C.  */
+  { "NSObject",		      0, 0, true, false, false, false,
+			      handle_nsobject_attribute, NULL },
   { NULL,                     0, 0, false, false, false, false, NULL, NULL }
 };
 
@@ -5124,6 +5128,41 @@  handle_patchable_function_entry_attribute (tree *, tree name, tree args,
   return NULL_TREE;
 }
 
+/* Handle a "NSObject" attributes; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_nsobject_attribute (tree *node, tree name, tree args,
+			   int /*flags*/, bool *no_add_attrs)
+{
+  *no_add_attrs = true;
+
+  /* This attribute only applies to typedefs (or field decls for properties),
+     we drop it otherwise - but warn about this if enabled.  */
+  if (TREE_CODE (*node) != TYPE_DECL && TREE_CODE (*node) != FIELD_DECL)
+    {
+      warning (OPT_WNSObject_attribute, "%qE attribute may be put on a"
+	       " typedef only; attribute is ignored", name);
+      return NULL_TREE;
+    }
+
+  /* The original implementation only allowed pointers to records, however
+     recent implementations also allow void *.  */
+  tree type = TREE_TYPE (*node);
+  if (!type || !POINTER_TYPE_P (type)
+      || (TREE_CODE (TREE_TYPE (type)) != RECORD_TYPE
+          && !VOID_TYPE_P (TREE_TYPE (type))))
+    {
+      error ("%qE attribute is for pointer types only", name);
+      return NULL_TREE;
+    }
+
+  tree t = tree_cons (name, args, TYPE_ATTRIBUTES (type));
+  TREE_TYPE (*node) = build_type_attribute_variant (type, t);
+
+  return NULL_TREE;
+}
+
 /* Attempt to partially validate a single attribute ATTR as if
    it were to be applied to an entity OPER.  */
 
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index ebd07cc8059..fe16357db85 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -256,6 +256,10 @@  U
 C ObjC C++ ObjC++ Joined Separate MissingArgError(macro name missing after %qs)
 -U<macro>	Undefine <macro>.
 
+WNSObject-attribute
+C ObjC C++ ObjC++ LTO Var(warn_nsobject_attribute) Warning Init(1)
+Warn if the NSObject attribute is applied to a non-typedef.
+
 Wabi
 C ObjC C++ ObjC++ LTO Var(warn_abi) Warning
 Warn about things that will change when compiling with an ABI-compliant compiler.
diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c
index 68d829fd773..8be4beadf3b 100644
--- a/gcc/objc/objc-act.c
+++ b/gcc/objc/objc-act.c
@@ -2476,9 +2476,14 @@  objc_compare_types (tree ltyp, tree rtyp, int argno, tree callee)
   if (!POINTER_TYPE_P (ltyp) || !POINTER_TYPE_P (rtyp))
     return false;
 
+  tree ltyp_attr, rtyp_attr;
   do
     {
-      ltyp = TREE_TYPE (ltyp);  /* Remove indirections.  */
+      /* Remove indirections, but keep the type attributes from the innermost
+	 pointer type, to check for NSObject.  */
+      ltyp_attr = TYPE_ATTRIBUTES (ltyp);
+      ltyp = TREE_TYPE (ltyp);
+      rtyp_attr = TYPE_ATTRIBUTES (rtyp);
       rtyp = TREE_TYPE (rtyp);
     }
   while (POINTER_TYPE_P (ltyp) && POINTER_TYPE_P (rtyp));
@@ -2523,17 +2528,23 @@  objc_compare_types (tree ltyp, tree rtyp, int argno, tree callee)
       return true;
     }
 
+  /* We might have void * with NSObject type attr.  */
+  bool l_NSObject_p = ltyp_attr && lookup_attribute ("NSObject", ltyp_attr);
+  bool r_NSObject_p = rtyp_attr && lookup_attribute ("NSObject", rtyp_attr);
+
   /* Past this point, we are only interested in ObjC class instances,
-     or 'id' or 'Class'.  */
-  if (TREE_CODE (ltyp) != RECORD_TYPE || TREE_CODE (rtyp) != RECORD_TYPE)
+     or 'id' or 'Class' (except if the user applied the NSObject type
+     attribute).  */
+  if ((TREE_CODE (ltyp) != RECORD_TYPE && !l_NSObject_p)
+      || (TREE_CODE (rtyp) != RECORD_TYPE && !r_NSObject_p))
     return false;
 
   if (!objc_is_object_id (ltyp) && !objc_is_class_id (ltyp)
-      && !TYPE_HAS_OBJC_INFO (ltyp))
+      && !TYPE_HAS_OBJC_INFO (ltyp) && !l_NSObject_p)
     return false;
 
   if (!objc_is_object_id (rtyp) && !objc_is_class_id (rtyp)
-      && !TYPE_HAS_OBJC_INFO (rtyp))
+      && !TYPE_HAS_OBJC_INFO (rtyp) && !r_NSObject_p)
     return false;
 
   /* Past this point, we are committed to returning 'true' to the caller
@@ -2567,12 +2578,15 @@  objc_compare_types (tree ltyp, tree rtyp, int argno, tree callee)
     rcls = NULL_TREE;
 
   /* If either type is an unqualified 'id', we're done.  This is because
-     an 'id' can be assigned to or from any type with no warnings.  */
+     an 'id' can be assigned to or from any type with no warnings.  When
+     the pointer has NSObject attribute, consider that to be equivalent.  */
   if (argno != -5)
     {
       if ((!lproto && objc_is_object_id (ltyp))
 	  || (!rproto && objc_is_object_id (rtyp)))
 	return true;
+      if (l_NSObject_p || r_NSObject_p)
+	return true;
     }
   else
     {
@@ -2580,7 +2594,7 @@  objc_compare_types (tree ltyp, tree rtyp, int argno, tree callee)
 	 general type of object, hence if you try to specialize an
 	 'NSArray *' (ltyp) property with an 'id' (rtyp) one, we need
 	 to warn.  */
-      if (!lproto && objc_is_object_id (ltyp))
+      if (!lproto && (objc_is_object_id (ltyp) || l_NSObject_p))
 	return true;
     }
 
@@ -8659,11 +8673,19 @@  objc_type_valid_for_messaging (tree type, bool accept_classes)
   if (!POINTER_TYPE_P (type))
     return false;
 
+  /* We will check for an NSObject type attribute  on the pointer if other
+     tests fail.  */
+  tree type_attr = TYPE_ATTRIBUTES (type);
+
   /* Remove the pointer indirection; don't remove more than one
      otherwise we'd consider "NSObject **" a valid type for messaging,
      which it isn't.  */
   type = TREE_TYPE (type);
 
+  /* We allow void * to have an NSObject type attr.  */
+  if (VOID_TYPE_P (type) && type_attr)
+    return lookup_attribute ("NSObject", type_attr) != NULL_TREE;
+
   if (TREE_CODE (type) != RECORD_TYPE)
     return false;
 
@@ -8676,6 +8698,9 @@  objc_type_valid_for_messaging (tree type, bool accept_classes)
   if (TYPE_HAS_OBJC_INFO (type))
     return true;
 
+  if (type_attr)
+    return lookup_attribute ("NSObject", type_attr) != NULL_TREE;
+
   return false;
 }
 
diff --git a/gcc/testsuite/obj-c++.dg/attributes/nsobject-01.mm b/gcc/testsuite/obj-c++.dg/attributes/nsobject-01.mm
new file mode 100644
index 00000000000..498fbc7e8fd
--- /dev/null
+++ b/gcc/testsuite/obj-c++.dg/attributes/nsobject-01.mm
@@ -0,0 +1,66 @@ 
+/* Test handling of the NSObject attribute.  */
+/*  { dg-additional-options "-fsyntax-only " } */
+
+typedef struct AnObj * __attribute__ ((NSObject)) AnObjRef;
+typedef struct AnObj * __attribute__ ((__NSObject__)) AnotherObjRef;
+
+/* We allow a void * to be labeled as NSObject.  */
+typedef void *  __attribute__((NSObject)) AnonRef;
+
+typedef struct AnObj * __attribute__((NSObject("foo"))) Bad; // { dg-error {wrong number of arguments specified for 'NSObject' attribute} }
+typedef struct AnObj * __attribute__((NSObject(42))) Wrong; // { dg-error {wrong number of arguments specified for 'NSObject' attribute} }
+
+/* Must be a pointer.  */
+typedef struct AnObj  __attribute__((NSObject)) BadRef; // { dg-error {'NSObject' attribute is for pointer types only} }
+
+typedef void * VPtr;
+
+@interface CheckAttrNSObject
+{
+@public
+  AnObjRef aor;
+  /* TODO: synthesize without pre-defined ivars.  */
+  VPtr obj_v;
+  int bar;
+  /* TODO: This should warn, even tho the property does not   */
+   __attribute__((NSObject)) struct AnObj *Thing;
+}
+
+@property(copy) AnObjRef aor;
+
+typedef struct AnObj * __attribute__((NSObject)) AnObjPtr3;
+@property (nonatomic, retain) AnObjPtr3 obj_3;
+
+@property (retain) __attribute__((NSObject)) VPtr obj_v;
+
+//@property (strong, nullable) AnObjPtr3 objp_4;
+
+@property(retain) __attribute__((NSObject)) int bar;
+ // { dg-error {'NSObject' attribute is for pointer types only} "" { target *-*-* } .-1 }
+ // { dg-error {'retain' attribute is only valid for Objective-C objects} "" { target *-*-* } .-2 }
+
+@end
+
+void foo ()
+{
+   __attribute__((NSObject)) struct AnObj *AnotherThing; // { dg-warning {'NSObject' attribute may be put on a typedef only; attribute is ignored} }
+}
+
+void
+setProperty(id self, id value)
+{
+  ((CheckAttrNSObject *)self)->aor = value;
+}
+
+id 
+getProperty(id self)
+{
+ return (id)((CheckAttrNSObject *)self)->aor;
+}
+
+@implementation CheckAttrNSObject
+@synthesize aor;
+@dynamic obj_3;
+@synthesize obj_v;
+@synthesize bar;
+@end // { dg-error {invalid conversion} }
diff --git a/gcc/testsuite/objc.dg/attributes/nsobject-01.m b/gcc/testsuite/objc.dg/attributes/nsobject-01.m
new file mode 100644
index 00000000000..5b568490391
--- /dev/null
+++ b/gcc/testsuite/objc.dg/attributes/nsobject-01.m
@@ -0,0 +1,66 @@ 
+/* Test handling of the NSObject attribute.  */
+/*  { dg-additional-options "-fsyntax-only " } */
+
+typedef struct AnObj * __attribute__ ((NSObject)) AnObjRef;
+typedef struct AnObj * __attribute__ ((__NSObject__)) AnotherObjRef;
+
+/* We allow a void * to be labeled as NSObject.  */
+typedef void *  __attribute__((NSObject)) AnonRef;
+
+typedef struct AnObj * __attribute__((NSObject("foo"))) Bad; // { dg-error {wrong number of arguments specified for 'NSObject' attribute} }
+typedef struct AnObj * __attribute__((NSObject(42))) Wrong; // { dg-error {wrong number of arguments specified for 'NSObject' attribute} }
+
+/* Must be a pointer.  */
+typedef struct AnObj  __attribute__((NSObject)) BadRef; // { dg-error {'NSObject' attribute is for pointer types only} }
+
+typedef void * VPtr;
+
+@interface CheckAttrNSObject
+{
+@public
+  AnObjRef aor;
+  /* TODO: synthesize without pre-defined ivars.  */
+  VPtr obj_v;
+  int bar;
+  /* TODO: This should warn, even tho the property does not   */
+   __attribute__((NSObject)) struct AnObj *Thing;
+}
+
+@property(copy) AnObjRef aor;
+
+typedef struct AnObj * __attribute__((NSObject)) AnObjPtr3;
+@property (nonatomic, retain) AnObjPtr3 obj_3;
+
+@property (retain) __attribute__((NSObject)) VPtr obj_v;
+
+//@property (strong, nullable) AnObjPtr3 objp_4;
+
+@property(retain) __attribute__((NSObject)) int bar;
+ // { dg-error {'NSObject' attribute is for pointer types only} "" { target *-*-* } .-1 }
+ // { dg-error {'retain' attribute is only valid for Objective-C objects} "" { target *-*-* } .-2 }
+
+@end
+
+void foo ()
+{
+   __attribute__((NSObject)) struct AnObj *AnotherThing; // { dg-warning {'NSObject' attribute may be put on a typedef only; attribute is ignored} }
+}
+
+void
+setProperty(id self, id value)
+{
+  ((CheckAttrNSObject *)self)->aor = value;
+}
+
+id 
+getProperty(id self)
+{
+ return (id)((CheckAttrNSObject *)self)->aor;
+}
+
+@implementation CheckAttrNSObject
+@synthesize aor;
+@dynamic obj_3;
+@synthesize obj_v;
+@synthesize bar; // { dg-warning {returning 'id' from a function with return type 'int'} }
+@end // { dg-warning {passing argument} }