Patchwork libobjc: review and complete the "Modern" typed selector API

login
register
mail settings
Submitter Nicola Pero
Date Dec. 24, 2010, 5 p.m.
Message ID <409E9DE2-BD6E-4247-A05A-3B018527E5FE@meta-innovation.com>
Download mbox | patch
Permalink /patch/76633/
State New
Headers show

Comments

Nicola Pero - Dec. 24, 2010, 5 p.m.
This patch is a review of libobjc's "Modern" API for typed selectors.   
We can't be compatible with the Apple runtime
because the Apple runtime doesn't have typed selectors, but we also  
can't really leave in place the "Traditional" GNU
Objective-C API with sel_get_typed_uid() etc. as it feels completely  
out of place in the context of the "Modern"
GNU Objective-C API.

This patch:

  * renames the GNU-specific 'sel_getType()' function to  
'sel_getTypeEncoding()' to be consistent with the rest of the Modern  
API,
which has 'method_getTypeEncoding()' and 'ivar_getTypeEncoding()'

  * adds 'sel_copyTypedSelectorList()' which is very general and  
allows you to build any query you want on typed selectors on
top of it.  It is consistent with the rest of the 'Modern' API which  
has got class_copyIvarList(), class_copyMethodList() etc. to get
access to raw lists of things.  Of course, like these other functions,  
it is relatively slow as it required a malloc, but it's always good
to have as a fallback.

  * adds 'sel_getTypedSelector()' which is similar to  
sel_get_any_typed_uid() or sel_get_typed_uid() etc. in the Traditional  
API.  It is a
fast way to do the most likely query with typed selectors, which is to  
get a typed selector by name (sel_registerName() doesn't do it
as it would register a new selector with no types;  
sel_registerTypedName() wouldn't do it either).

With these changes, I think we can safely deprecate the old  
Traditional API for typed selectors. :-)

Committed to trunk.

Thanks

  /** Implementation: the following functions are in objects.c.  */

Patch

Index: ChangeLog
===================================================================
--- ChangeLog   (revision 168211)
+++ ChangeLog   (working copy)
@@ -1,3 +1,11 @@ 
+2010-12-24  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc.dg/gnu-api-2-sel.m: Updated for renaming of sel_getType  
to
+       sel_getTypeEncoding.  Test that sel_getTypeEncoding returns NULL
+       when called with a NULL argument.  Added test for
+       sel_copyTypedSelectorList and sel_getTypedSelector.
+       * obj-c++.dg/gnu-api-2-sel.mm: Same changes.
+
  2010-12-22  Sebastian Pop  <sebastian.pop@amd.com>

         PR tree-optimization/46758
Index: objc.dg/gnu-api-2-sel.m
===================================================================
--- objc.dg/gnu-api-2-sel.m     (revision 168228)
+++ objc.dg/gnu-api-2-sel.m     (working copy)
@@ -35,11 +35,13 @@ 
  { id variable_ivar; }
  - (void) setVariable: (id)value;
  - (id) variable;
+- (void) method;
  @end

  @implementation MySubClass
  - (void) setVariable: (id)value { variable_ivar = value; }
  - (id) variable { return variable_ivar; }
+- (void) method { return; }
  @end


@@ -47,6 +49,30 @@ 
  {
    /* Functions are tested in alphabetical order.  */

+  printf ("Testing sel_copyTypedSelectorList ()...\n");
+  {
+    unsigned int count;
+    SEL * list = sel_copyTypedSelectorList ("method", &count);
+
+    /* There should only be two, since 'method' is referenced twice,
+       once with types and once without (in this very test).  */
+    if (count != 2)
+      abort ();
+
+    /* Check that both selectors are not-NULL, and have the correct
+       name.  We use @selector() here, which wouldn't really be
+       needed, just to register a second, untyped selector with name
+       'method'.  */
+    if (strcmp (sel_getName (list[0]), sel_getName (@selector  
(method))) != 0)
+      abort ();
+
+    if (strcmp (sel_getName (list[1]), sel_getName (@selector  
(method))) != 0)
+      abort ();
+
+    if (list[2] != NULL)
+      abort ();
+  }
+
    printf ("Testing sel_getName () ...\n");
    {
      if (strcmp (sel_getName (@selector (variable)), "variable") != 0)
@@ -56,17 +82,53 @@ 
        abort ();
    }

-  printf ("Testing sel_getType () ...\n");
+  printf ("Testing sel_getTypeEncoding () ...\n");
    {
      /* Get a selector from a real class, so it has interesting
         types.  */
      Method method = class_getInstanceMethod (objc_getClass  
("MySubClass"),
                                              @selector (variable));

-    if (strcmp (sel_getType (method_getName (method)),  
method_getTypeEncoding (method)) != 0)
+    if (strcmp (sel_getTypeEncoding (method_getName (method)),
+               method_getTypeEncoding (method)) != 0)
        abort ();
+
+    if (sel_getTypeEncoding (NULL) != NULL)
+      abort ();
    }
+  printf ("Testing sel_getTypedSelector () ...\n");
+  {
+    /* First try with a selector where we know that a typed one has
+       been registered.  */
+    SEL selector = sel_getTypedSelector ("variable");
+
+    if (selector == NULL)
+      abort ();
+
+    if (sel_getTypeEncoding (selector) == NULL)
+      abort ();
+
+    /* Now try a selector which was never registered.  */
+    selector = sel_getTypedSelector ("not_registered");
+
+    if (selector != NULL)
+      abort ();
+
+    /* Now try registering a selector with no types.  The following
+       line is just a way to have an unused '@selector()' expression
+       without the compiler complaining.  */
+    if (@selector (registered_with_no_types) == NULL)
+      abort ();
+
+    /* Try getting it.  Nothing should be returned because it is
+       untyped.  */
+    selector = sel_getTypedSelector ("registered_with_no_types");
+
+    if (selector != NULL)
+      abort ();
+  }
+
    printf ("Testing sel_getUid () ...\n");
    {
      if (strcmp (sel_getName (sel_getUid ("myMethod")), "myMethod") ! 
= 0)
@@ -95,7 +157,7 @@ 
      if (strcmp (sel_getName (selector), "aMethod") != 0)
        abort ();

-    if (strcmp (sel_getType (selector), types) != 0)
+    if (strcmp (sel_getTypeEncoding (selector), types) != 0)
        abort ();
    }
  Index: obj-c++.dg/gnu-api-2-sel.mm
===================================================================
--- obj-c++.dg/gnu-api-2-sel.mm (revision 168211)
+++ obj-c++.dg/gnu-api-2-sel.mm (working copy)
@@ -35,11 +35,13 @@ 
  { id variable_ivar; }
  - (void) setVariable: (id)value;
  - (id) variable;
+- (void) method;
  @end

  @implementation MySubClass
  - (void) setVariable: (id)value { variable_ivar = value; }
  - (id) variable { return variable_ivar; }
+- (void) method { return; }
  @end


@@ -47,6 +49,30 @@ 
  {
    /* Functions are tested in alphabetical order.  */

+  std::cout << "Testing sel_copyTypedSelectorList ()...\n";
+  {
+    unsigned int count;
+    SEL * list = sel_copyTypedSelectorList ("method", &count);
+
+    /* There should only be two, since 'method' is referenced twice,
+       once with types and once without (in this very test).  */
+    if (count != 2)
+      abort ();
+
+    /* Check that both selectors are not-NULL, and have the correct
+       name.  We use @selector() here, which wouldn't really be
+       needed, just to register a second, untyped selector with name
+       'method'.  */
+    if (std::strcmp (sel_getName (list[0]), sel_getName (@selector  
(method))) != 0)
+      abort ();
+
+    if (std::strcmp (sel_getName (list[1]), sel_getName (@selector  
(method))) != 0)
+      abort ();
+
+    if (list[2] != NULL)
+      abort ();
+  }
+
    std::cout << "Testing sel_getName () ...\n";
    {
      if (std::strcmp (sel_getName (@selector (variable)),  
"variable") != 0)
@@ -56,17 +82,53 @@ 
        abort ();
    }

-  std::cout << "Testing sel_getType () ...\n";
+  std::cout << "Testing sel_getTypeEncoding () ...\n";
    {
      /* Get a selector from a real class, so it has interesting
         types.  */
      Method method = class_getInstanceMethod (objc_getClass  
("MySubClass"),
                                              @selector (variable));

-    if (std::strcmp (sel_getType (method_getName (method)),  
method_getTypeEncoding (method)) != 0)
+    if (std::strcmp (sel_getTypeEncoding (method_getName (method)),
+               method_getTypeEncoding (method)) != 0)
        abort ();
+
+    if (sel_getTypeEncoding (NULL) != NULL)
+      abort ();
    }

+  std::cout << "Testing sel_getTypedSelector () ...\n";
+  {
+    /* First try with a selector where we know that a typed one has
+       been registered.  */
+    SEL selector = sel_getTypedSelector ("variable");
+
+    if (selector == NULL)
+      abort ();
+
+    if (sel_getTypeEncoding (selector) == NULL)
+      abort ();
+
+    /* Now try a selector which was never registered.  */
+    selector = sel_getTypedSelector ("not_registered");
+
+    if (selector != NULL)
+      abort ();
+
+    /* Now try registering a selector with no types.  The following
+       line is just a wayx to have an unused '@selector()' expression
+       without the compiler complaining.  */
+    if (@selector (registered_with_no_types) == NULL)
+      abort ();
+
+    /* Try getting it.  Nothing should be returned because it is
+       untyped.  */
+    selector = sel_getTypedSelector ("registered_with_no_types");
+
+    if (selector != NULL)
+      abort ();
+  }
+
    std::cout << "Testing sel_getUid () ...\n";
    {
      if (std::strcmp (sel_getName (sel_getUid ("myMethod")),  
"myMethod") != 0)
@@ -91,11 +153,11 @@ 
                                                 (objc_getClass  
("MySubClass"),
                                                  @selector  
(variable)));
      SEL selector = sel_registerTypedName ("aMethod", types);
-
+
      if (std::strcmp (sel_getName (selector), "aMethod") != 0)
        abort ();

-    if (std::strcmp (sel_getType (selector), types) != 0)
+    if (std::strcmp (sel_getTypeEncoding (selector), types) != 0)
        abort ();
    }
Index: selector.c
===================================================================
--- selector.c  (revision 168211)
+++ selector.c  (working copy)
@@ -31,6 +31,7 @@  see the files COPYING3 and COPYING.RUNTI
  #include "objc-private/runtime.h"
  #include "objc-private/sarray.h"
  #include "objc-private/selector.h"
+#include <stdlib.h>                    /* For malloc.  */

  /* Initial selector hash table size. Value doesn't matter much.  */
  #define SELECTOR_HASH_SIZE 128
@@ -250,7 +251,11 @@  sel_types_match (const char *t1, const c
    return NO;
  }

-/* Return selector representing name.  */
+/* Return selector representing name.  In the Modern API, you'd
+   normally use sel_registerTypedName() for this, which does the same
+   but would register the selector with the runtime if not registered
+   yet (if you only want to check for selectors without registering,
+   use sel_copyTypedSelectorList()).  */
  SEL
  sel_get_typed_uid (const char *name, const char *types)
  {
@@ -290,7 +295,8 @@  sel_get_typed_uid (const char *name, con
  }

  /* Return selector representing name; prefer a selector with non-NULL
-   type.  */
+   type.  In the Modern API, sel_getTypedSelector() is similar but
+   returns NULL if a typed selector couldn't be found.  */
  SEL
  sel_get_any_typed_uid (const char *name)
  {
@@ -347,6 +353,91 @@  sel_get_any_uid (const char *name)
    return (SEL) l->head;
  }
  +SEL
+sel_getTypedSelector (const char *name)
+{
+  sidx i;
+  objc_mutex_lock (__objc_runtime_mutex);
+
+  /* Look for a typed selector.  */
+  i = (sidx) objc_hash_value_for_key (__objc_selector_hash, name);
+  if (i != 0)
+    {
+      struct objc_list *l;
+
+      for (l = (struct objc_list *) sarray_get_safe  
(__objc_selector_array, i);
+          l; l = l->tail)
+       {
+         SEL s = (SEL) l->head;
+         if (s->sel_types)
+           {
+             objc_mutex_unlock (__objc_runtime_mutex);
+             return s;
+           }
+       }
+    }
+
+  /* No typed selector found.  Return NULL.  */
+  objc_mutex_unlock (__objc_runtime_mutex);
+  return 0;
+}
+
+SEL *
+sel_copyTypedSelectorList (const char *name, unsigned int  
*numberOfReturnedSelectors)
+{
+  unsigned int count = 0;
+  SEL *returnValue = NULL;
+  sidx i;
+
+  if (name == NULL)
+    {
+      if (numberOfReturnedSelectors)
+       *numberOfReturnedSelectors = 0;
+      return NULL;
+    }
+
+  objc_mutex_lock (__objc_runtime_mutex);
+
+  /* Count how many selectors we have.  */
+  i = (sidx) objc_hash_value_for_key (__objc_selector_hash, name);
+  if (i != 0)
+    {
+      struct objc_list *selector_list = NULL;
+      selector_list = (struct objc_list *) sarray_get_safe  
(__objc_selector_array, i);
+
+      /* Count how many selectors we have.  */
+      {
+       struct objc_list *l;
+       for (l = selector_list; l; l = l->tail)
+         count++;
+      }
+
+      if (count != 0)
+       {
+         /* Allocate enough memory to hold them.  */
+         returnValue = (SEL *)(malloc (sizeof (SEL) * (count + 1)));
+
+         /* Copy the selectors.  */
+         {
+           unsigned int j;
+           for (j = 0; j < count; j++)
+             {
+               returnValue[j] = (SEL)(selector_list->head);
+               selector_list = selector_list->tail;
+             }
+           returnValue[j] = NULL;
+         }
+       }
+    }
+
+  objc_mutex_unlock (__objc_runtime_mutex);
+
+  if (numberOfReturnedSelectors)
+    *numberOfReturnedSelectors = count;
+
+  return returnValue;
+}
+
  /* Get the name of a selector.  If the selector is unknown, the empty
     string "" is returned.  */
  const char *sel_getName (SEL selector)
@@ -382,7 +473,7 @@  sel_is_mapped (SEL selector)
    return ((idx > 0) && (idx <= __objc_selector_max_index));
  }

-const char *sel_getType (SEL selector)
+const char *sel_getTypeEncoding (SEL selector)
  {
    if (selector)
      return selector->sel_types;
@@ -393,7 +484,7 @@  const char *sel_getType (SEL selector)
  /* Traditional GNU Objective-C Runtime API.  */
  const char *sel_get_type (SEL selector)
  {
-  return sel_getType (selector);
+  return sel_getTypeEncoding (selector);
  }

  /* The uninstalled dispatch table.  */
Index: ChangeLog
===================================================================
--- ChangeLog   (revision 168215)
+++ ChangeLog   (working copy)
@@ -1,3 +1,13 @@ 
+2010-12-24  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc/runtime.h (sel_getType): Renamed to sel_getTypeEncoding  
to
+       be consistent with method_getTypeEncoding and
+       ivar_getTypeEncoding.
+       (sel_copyTypedSelectorList, sel_getTypedSelector): New.
+       * selector.c (sel_getType): Renamed to sel_getTypeEncoding.
+       (sel_copyTypedSelectorList, sel_getTypedSelector): New.
+       (sel_get_type): Updated call to sel_getType.
+
  2010-12-23  Nicola Pero  <nicola.pero@meta-innovation.com>

         * init.c (create_tree_of_subclasses_inherited_from): Updated
Index: objc/runtime.h
===================================================================
--- objc/runtime.h      (revision 168211)
+++ objc/runtime.h      (working copy)
@@ -175,12 +175,13 @@  object_getClass (id object)
     "<null selector>".  */
  objc_EXPORT const char *sel_getName (SEL selector);

-/* Return the type of a given selector.
+/* Return the type of a given selector.  Return NULL if selector is
+   NULL.

     Compatibility Note: the Apple/NeXT runtime has untyped selectors,
     so it does not have this function, which is specific to the GNU
     Runtime.  */
-objc_EXPORT const char *sel_getType (SEL selector);
+objc_EXPORT const char *sel_getTypeEncoding (SEL selector);

  /* This is the same as sel_registerName ().  Please use
     sel_registerName () instead.  */
@@ -188,11 +189,16 @@  objc_EXPORT SEL sel_getUid (const char *

  /* Register a selector with a given name (but unspecified types).  If
     you know the types, it is better to call sel_registerTypedName().
-   If a selector with this name already exists, it is returned.  */
+   If a selector with this name and no types already exists, it is
+   returned.  Note that this function should really be called
+   'objc_registerSelector'.  */
  objc_EXPORT SEL sel_registerName (const char *name);

  /* Register a selector with a given name and types.  If a selector
-   with this name and types already exists, it is returned.
+   with this name and types already exists, it is returned.  Note that
+   this function should really be called 'objc_registerTypedSelector',
+   and it's called 'sel_registerTypedName' only for consistency with
+   'sel_registerName'.

     Compatibility Note: the Apple/NeXT runtime has untyped selectors,
     so it does not have this function, which is specific to the GNU
@@ -203,6 +209,37 @@  objc_EXPORT SEL sel_registerTypedName (c
     if not.  */
  objc_EXPORT BOOL sel_isEqual (SEL first_selector, SEL  
second_selector);

+/* Return all the selectors with the supplied name.  In the GNU
+   runtime, selectors are typed and there may be multiple selectors
+   with the same name but a different type.  The return value of the
+   function is a pointer to an area, allocated with malloc(), that
+   contains all the selectors with the supplier name known to the
+   runtime.  The list is terminated by NULL.  Optionally, if you pass
+   a non-NULL 'numberOfReturnedSelectors' pointer, the unsigned int
+   that it points to will be filled with the number of selectors
+   returned.
+
+   Compatibility Note: the Apple/NeXT runtime has untyped selectors,
+   so it does not have this function, which is specific to the GNU
+   Runtime.  */
+objc_EXPORT SEL * sel_copyTypedSelectorList (const char *name,
+                                            unsigned int  
*numberOfReturnedSelectors);
+
+/* Return a selector with name 'name' and a non-zero type encoding, if
+   any such selector is registered with the runtime.  If there is no
+   such selector, NULL is returned.
+
+   This is useful if you have the name of the selector, and would
+   really like to get a selector for it that includes the type
+   encoding.  Unfortunately, if the program contains multiple selector
+   with the same name but different types, sel_getTypedSelector
+   returns a random one of them, which may not be the right one.
+
+   Compatibility Note: the Apple/NeXT runtime has untyped selectors,
+   so it does not have this function, which is specific to the GNU
+   Runtime.  */
+objc_EXPORT SEL sel_getTypedSelector (const char *name);
+