Patchwork =?UTF-8?Q?ObjC/ObjC++=20-=20remove=20objc=5Finherit=5Fcode?=

login
register
mail settings
Submitter Nicola Pero
Date Oct. 20, 2010, 9:29 p.m.
Message ID <1287610193.06048787@192.168.2.230>
Download mbox | patch
Permalink /patch/68482/
State New
Headers show

Comments

Nicola Pero - Oct. 20, 2010, 9:29 p.m.
This patch simplifies the parsing of ObjC / ObjC++ method "types" (ie, the '+' or '-' 
at the beginning of a method declaration or definition).

If you look at Objective-C, for example, before this patch, c_parser_objc_method_type
would parse the '+' or '-' and return "PLUS_EXPR" or "MINUS_EXPR" to the caller.  
The caller would then call objc_set_method_type which would convert the "PLUS_EXPR" 
and "MINUS_EXPR" into "CLASS_METHOD_DECL" or "INSTANCE_METHOD_DECL", and store these 
into a global (inside objc-act.c) static variable called objc_inherit_code.
Finally, when the parser [a couple of lines later ;-)] called objc_start_method_definition
or objc_add_method_declaration, the global variable would be checked to determine if we are
parsing a class or instance method.

This patch simplifies it all.  c_parser_objc_method_type now returns 'true' for a class
method, and 'false' for an instance method.  The parser keeps that information in a local
bool 'is_class_method' variable, and when it calls objc_start_method_definition or
objc_add_method_declaration a few lines later, it passes is_class_method as an argument.
objc_inehrit_code is gone, the PLUS_EXPR and MINUS_EXPR are gone, objc_set_method_type 
is gone, the conversions are gone and arguments are passed to functions as arguments and
not via global variables.  Not only it's so much easier to understand, but it's also (very 
very slightly) faster. :-)

Ok to commit to trunk ?

Thanks

PS: there is nothing left to merge from the FSF apple/trunk branch in this code, so I'm not
interfering with anything.  But I want to start cleaning things up.
Mike Stump - Oct. 20, 2010, 10:15 p.m.
On Oct 20, 2010, at 2:29 PM, Nicola Pero wrote:
> This patch simplifies the parsing of ObjC / ObjC++ method "types" (ie, the '+' or '-' 
> at the beginning of a method declaration or definition).

> Ok to commit to trunk ?

Ok.

Patch

Index: c-family/ChangeLog
===================================================================
--- c-family/ChangeLog  (revision 165731)
+++ c-family/ChangeLog  (working copy)
@@ -1,5 +1,16 @@ 
 2010-10-20  Nicola Pero  <nicola.pero@meta-innovation.com>
 
+       * c-common.h (objc_set_method_type): Removed.
+       (objc_add_method_declaration): Added boolean argument.
+       (objc_start_method_definition): Same change.
+       (objc_build_method_signature): Same change.
+       * stub-objc.c (objc_set_method_type): Removed.
+       (objc_add_method_declaration): Added boolean argument.
+       (objc_start_method_definition): Same change.
+       (objc_build_method_signature): Same change.
+
+2010-10-20  Nicola Pero  <nicola.pero@meta-innovation.com>
+
        * c-common.h (finish_file): Removed.
        (objc_write_global_declarations): New.
        * c-opts.c (c_common_parse_file): Do not call finish_file.
Index: c-family/c-common.h
===================================================================
--- c-family/c-common.h (revision 165731)
+++ c-family/c-common.h (working copy)
@@ -1020,10 +1020,9 @@  extern void objc_start_category_implementation (tr
 extern void objc_continue_implementation (void);
 extern void objc_finish_implementation (void);
 extern void objc_set_visibility (objc_ivar_visibility_kind);
-extern void objc_set_method_type (enum tree_code);
-extern tree objc_build_method_signature (tree, tree, tree, bool);
-extern void objc_add_method_declaration (tree, tree);
-extern bool objc_start_method_definition (tree, tree);
+extern tree objc_build_method_signature (bool, tree, tree, tree, bool);
+extern void objc_add_method_declaration (bool, tree, tree);
+extern bool objc_start_method_definition (bool, tree, tree);
 extern void objc_finish_method_definition (tree);
 extern void objc_add_instance_variable (tree);
 extern tree objc_build_keyword_decl (tree, tree, tree, tree);
Index: c-family/stub-objc.c
===================================================================
--- c-family/stub-objc.c        (revision 165731)
+++ c-family/stub-objc.c        (working copy)
@@ -179,11 +179,6 @@  objc_set_visibility (objc_ivar_visibility_kind ARG
 }
 
 void
-objc_set_method_type (enum tree_code ARG_UNUSED (code))
-{
-}
-
-void
 objc_start_class_implementation (tree ARG_UNUSED (name),
                                 tree ARG_UNUSED (super))
 {
@@ -211,13 +206,15 @@  objc_finish_implementation (void)
 }
 
 void
-objc_add_method_declaration (tree ARG_UNUSED (signature),
+objc_add_method_declaration (bool ARG_UNUSED (is_class_method),
+                            tree ARG_UNUSED (signature),
                             tree ARG_UNUSED (attributes))
 {
 }
 
 bool
-objc_start_method_definition (tree ARG_UNUSED (signature),
+objc_start_method_definition (bool ARG_UNUSED (is_class_method),
+                             tree ARG_UNUSED (signature),
                              tree ARG_UNUSED (attributes))
 {
   return true;
@@ -244,7 +241,8 @@  objc_build_keyword_decl (tree ARG_UNUSED (selector
 }
 
 tree
-objc_build_method_signature (tree ARG_UNUSED (rettype),
+objc_build_method_signature (bool ARG_UNUSED (is_class_method),
+                            tree ARG_UNUSED (rettype),
                             tree ARG_UNUSED (selectors),
                             tree ARG_UNUSED (optparms),
                             bool ARG_UNUSED (ellipsis))
Index: objc/objc-act.c
===================================================================
--- objc/objc-act.c     (revision 165732)
+++ objc/objc-act.c     (working copy)
@@ -392,7 +392,6 @@  struct imp_entry *imp_list = 0;
 int imp_count = 0;     /* `@implementation' */
 int cat_count = 0;     /* `@category' */
 
-enum tree_code objc_inherit_code;
 objc_ivar_visibility_kind objc_ivar_visibility;
 
 /* Use to generate method labels.  */
@@ -1278,24 +1277,20 @@  build_property_reference (tree property, tree id)
   return getter;
 }
 
-void
-objc_set_method_type (enum tree_code type)
-{
-  objc_inherit_code = (type == PLUS_EXPR
-                      ? CLASS_METHOD_DECL
-                      : INSTANCE_METHOD_DECL);
-}
-
 tree
-objc_build_method_signature (tree rettype, tree selector,
+objc_build_method_signature (bool is_class_method, tree rettype, tree selector,
                             tree optparms, bool ellipsis)
 {
-  return build_method_decl (objc_inherit_code, rettype, selector,
-                           optparms, ellipsis);
+  if (is_class_method)
+    return build_method_decl (CLASS_METHOD_DECL, rettype, selector,
+                             optparms, ellipsis);
+  else
+    return build_method_decl (INSTANCE_METHOD_DECL, rettype, selector,
+                             optparms, ellipsis);
 }
 
 void
-objc_add_method_declaration (tree decl, tree attributes)
+objc_add_method_declaration (bool is_class_method, tree decl, tree attributes)
 {
   if (!objc_interface_context)
     {
@@ -1309,7 +1304,7 @@  void
   objc_decl_method_attributes (&decl, attributes, 0);
   objc_add_method (objc_interface_context,
                   decl,
-                  objc_inherit_code == CLASS_METHOD_DECL,
+                  is_class_method,
                   objc_method_optional_flag);
 }
 
@@ -1317,7 +1312,7 @@  void
    'false' if not (because we are outside an @implementation context).
 */
 bool
-objc_start_method_definition (tree decl, tree attributes)
+objc_start_method_definition (bool is_class_method, tree decl, tree attributes)
 {
   if (!objc_implementation_context)
     {
@@ -1338,7 +1333,7 @@  bool
   objc_decl_method_attributes (&decl, attributes, 0);
   objc_add_method (objc_implementation_context,
                   decl,
-                  objc_inherit_code == CLASS_METHOD_DECL, 
+                  is_class_method,
                   /* is optional */ false);
   start_method_def (decl);
   return true;
@@ -5179,17 +5174,18 @@  objc_generate_cxx_ctor_or_dtor (bool dtor)
   /* - (id) .cxx_construct { ... return self; } */
   /* - (void) .cxx_construct { ... }            */
 
-  objc_set_method_type (MINUS_EXPR);
   objc_start_method_definition
-   (objc_build_method_signature (build_tree_list (NULL_TREE,
-                                                 dtor
-                                                 ? void_type_node
-                                                 : objc_object_type),
-                                get_identifier (dtor
-                                                ? TAG_CXX_DESTRUCT
-                                                : TAG_CXX_CONSTRUCT),
-                                make_node (TREE_LIST),
-                                false), NULL);
+    (false /* is_class_method */,
+     objc_build_method_signature (false /* is_class_method */,
+                                 build_tree_list (NULL_TREE,
+                                                  dtor
+                                                  ? void_type_node
+                                                  : objc_object_type),
+                                 get_identifier (dtor
+                                                 ? TAG_CXX_DESTRUCT
+                                                 : TAG_CXX_CONSTRUCT),
+                                 make_node (TREE_LIST),
+                                 false), NULL);
   body = begin_function_body ();
   compound_stmt = begin_compound_stmt (0);
 
@@ -6498,8 +6494,7 @@  synth_id_with_class_suffix (const char *preamble,
 
 /* If type is empty or only type qualifiers are present, add default
    type of id (otherwise grokdeclarator will default to int).  */
-
-static tree
+static inline tree
 adjust_type_for_id_default (tree type)
 {
   if (!type)
@@ -6557,7 +6552,6 @@  objc_build_keyword_decl (tree key_name, tree arg_t
 }
 
 /* Given a chain of keyword_decl's, synthesize the full keyword selector.  */
-
 static tree
 build_keyword_selector (tree selector)
 {
@@ -8660,9 +8654,8 @@  objc_synthesize_getter (tree klass, tree class_met
   if (!decl)
     return;
 
-  objc_inherit_code = INSTANCE_METHOD_DECL;
   /* For now no attributes.  */
-  objc_start_method_definition (copy_node (decl), NULL_TREE);
+  objc_start_method_definition (false /* is_class_method */, copy_node (decl), NULL_TREE);
 
   body = c_begin_compound_stmt (true);
   /* return self->_property_name; */
@@ -8720,9 +8713,8 @@  objc_synthesize_setter (tree klass, tree class_met
   if (!decl)
     return;
 
-  objc_inherit_code = INSTANCE_METHOD_DECL;
   /* For now, no attributes.  */
-  objc_start_method_definition (copy_node (decl), NULL_TREE);
+  objc_start_method_definition (false /* is_class_method */, copy_node (decl), NULL_TREE);
 
   body = c_begin_compound_stmt (true);
   /* _property_name = _value; */
Index: objc/ChangeLog
===================================================================
--- objc/ChangeLog      (revision 165732)
+++ objc/ChangeLog      (working copy)
@@ -1,3 +1,20 @@ 
+2010-10-20    <nicola@localhost.localdomain>
+
+       * objc-act.h (objc_inherit_code): Removed.
+       * objc-act.c (objc_inherit_code): Removed.
+       (objc_set_method_type): Removed.
+       (objc_build_method_signature): Added is_class_method argument.
+       Use it instead of the global objc_inherit_code variable.
+       (objc_add_method_declaration): Same change.
+       (objc_start_method_definition): Same change.
+       (objc_generate_cxx_ctor_or_dtor): Updated call to
+       objc_start_method_definition.  Do not call objc_set_method_type.
+       (adjust_type_for_id_default): Mark as inline.
+       (objc_synthesize_getter): Updated call to
+       objc_start_method_definition.  Do not set objc_inherit_code.
+       (objc_synthesize_setter): Updated call to
+       objc_start_method_definition.  Do not set objc_inherit_code.    
+       
 2010-10-20  Nicola Pero  <nicola.pero@meta-innovation.com>
 
        Merge from 'apple/trunk' branch on FSF servers.  Obvious updates
Index: objc/objc-act.h
===================================================================
--- objc/objc-act.h     (revision 165732)
+++ objc/objc-act.h     (working copy)
@@ -176,7 +176,6 @@  extern GTY(()) struct imp_entry *imp_list;
 extern GTY(()) int imp_count;  /* `@implementation' */
 extern GTY(()) int cat_count;  /* `@category' */
 
-extern GTY(()) enum tree_code objc_inherit_code;
 extern GTY(()) objc_ivar_visibility_kind objc_ivar_visibility;
 
 /* Objective-C/Objective-C++ global tree enumeration.  */
Index: ChangeLog
===================================================================
--- ChangeLog   (revision 165731)
+++ ChangeLog   (working copy)
@@ -1,3 +1,16 @@ 
+2010-10-20  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * parser.c (c_parser_objc_method_type): Mark inline.  Return a
+       bool instead of a tree.
+       (c_parser_objc_method_decl): Added bool argument.  Updated call to
+       objc_build_method_signature.
+       (c_parser_objc_method_definition): Do not call
+       objc_set_method_type.  Updated calls to c_parser_objc_method_type,
+       c_parser_objc_method_decl and objc_start_method_definition.
+       (c_parser_objc_methodproto): Do not call objc_set_method_type.
+       Updated calls to c_parser_objc_method_type,
+       c_parser_objc_method_decl and objc_add_method_declaration.
+       
 2010-10-20  Eric Botcazou  <ebotcazou@adacore.com>
 
        * tree-optimize.c (execute_fixup_cfg): Purge dead abnormal call edges
Index: cp/ChangeLog
===================================================================
--- cp/ChangeLog        (revision 165731)
+++ cp/ChangeLog        (working copy)
@@ -1,3 +1,14 @@ 
+2010-10-20  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * parser.c (cp_parser_objc_method_type): Mark inline.  Return a
+       bool instead of calling objc_set_method_type.
+       (cp_parser_objc_method_signature): Updated calls to
+       cp_parser_objc_method_type and to objc_build_method_signature.
+       (cp_parser_objc_method_prototype_list): Updated calls to
+       objc_add_method_declaration.  Use token->type to determine if it
+       is a class method or not.       
+       (cp_parser_objc_method_definition_list): Same change.
+       
 2010-10-20  Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
 
        PR c++/46056
Index: cp/parser.c
===================================================================
--- cp/parser.c (revision 165731)
+++ cp/parser.c (working copy)
@@ -21394,15 +21394,16 @@  cp_parser_objc_visibility_spec (cp_parser* parser)
   cp_lexer_consume_token (parser->lexer);
 }
 
-/* Parse an Objective-C method type.  */
+/* Parse an Objective-C method type.  Return 'true' if it is a class
+   (+) method, and 'false' if it is an instance (-) method.  */
 
-static void
+static inline bool
 cp_parser_objc_method_type (cp_parser* parser)
 {
-  objc_set_method_type
-   (cp_lexer_consume_token (parser->lexer)->type == CPP_PLUS
-    ? PLUS_EXPR
-    : MINUS_EXPR);
+  if (cp_lexer_consume_token (parser->lexer)->type == CPP_PLUS)
+    return true;
+  else
+    return false;
 }
 
 /* Parse an Objective-C protocol qualifier.  */
@@ -21688,8 +21689,9 @@  cp_parser_objc_method_signature (cp_parser* parser
 {
   tree rettype, kwdparms, optparms;
   bool ellipsis = false;
+  bool is_class_method;
 
-  cp_parser_objc_method_type (parser);
+  is_class_method = cp_parser_objc_method_type (parser);
   rettype = cp_parser_objc_typename (parser);
   *attributes = NULL_TREE;
   kwdparms = cp_parser_objc_method_keyword_params (parser, attributes);
@@ -21699,7 +21701,7 @@  cp_parser_objc_method_signature (cp_parser* parser
   if (optparms == error_mark_node)
     return error_mark_node;
 
-  return objc_build_method_signature (rettype, kwdparms, optparms, ellipsis);
+  return objc_build_method_signature (is_class_method, rettype, kwdparms, optparms, ellipsis);
 }
 
 static bool
@@ -21734,6 +21736,11 @@  cp_parser_objc_method_prototype_list (cp_parser* p
       if (token->type == CPP_PLUS || token->type == CPP_MINUS)
        {
          tree attributes, sig;
+         bool is_class_method;
+         if (token->type == CPP_PLUS)
+           is_class_method = true;
+         else
+           is_class_method = false;
          sig = cp_parser_objc_method_signature (parser, &attributes);
          if (sig == error_mark_node)
            {
@@ -21741,7 +21748,7 @@  cp_parser_objc_method_prototype_list (cp_parser* p
              token = cp_lexer_peek_token (parser->lexer);
              continue;
            }
-         objc_add_method_declaration (sig, attributes);
+         objc_add_method_declaration (is_class_method, sig, attributes);
          cp_parser_consume_semicolon_at_end_of_statement (parser);
        }
       else if (token->keyword == RID_AT_PROPERTY)
@@ -21781,6 +21788,11 @@  cp_parser_objc_method_definition_list (cp_parser*
        {
          cp_token *ptk;
          tree sig, attribute;
+         bool is_class_method;
+         if (token->type == CPP_PLUS)
+           is_class_method = true;
+         else
+           is_class_method = false;
          push_deferring_access_checks (dk_deferred);
          sig = cp_parser_objc_method_signature (parser, &attribute);
          if (sig == error_mark_node)
@@ -21789,7 +21801,7 @@  cp_parser_objc_method_definition_list (cp_parser*
              token = cp_lexer_peek_token (parser->lexer);
              continue;
            }
-         objc_start_method_definition (sig, attribute);
+         objc_start_method_definition (is_class_method, sig, attribute);
 
          /* For historical reasons, we accept an optional semicolon.  */
          if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
Index: c-parser.c
===================================================================
--- c-parser.c  (revision 165731)
+++ c-parser.c  (working copy)
@@ -1067,11 +1067,11 @@  static void c_parser_objc_class_instance_variables
 static void c_parser_objc_class_declaration (c_parser *);
 static void c_parser_objc_alias_declaration (c_parser *);
 static void c_parser_objc_protocol_definition (c_parser *, tree);
-static enum tree_code c_parser_objc_method_type (c_parser *);
+static bool c_parser_objc_method_type (c_parser *);
 static void c_parser_objc_method_definition (c_parser *);
 static void c_parser_objc_methodprotolist (c_parser *);
 static void c_parser_objc_methodproto (c_parser *);
-static tree c_parser_objc_method_decl (c_parser *, tree *);
+static tree c_parser_objc_method_decl (c_parser *, bool, tree *);
 static tree c_parser_objc_type_name (c_parser *);
 static tree c_parser_objc_protocol_refs (c_parser *);
 static void c_parser_objc_try_catch_statement (c_parser *);
@@ -6889,19 +6889,21 @@  c_parser_objc_protocol_definition (c_parser *parse
    objc-method-type:
      +
      -
+
+   Return true if it is a class method (+) and false if it is
+   an instance method (-).
 */
-
-static enum tree_code
+static inline bool
 c_parser_objc_method_type (c_parser *parser)
 {
   switch (c_parser_peek_token (parser)->type)
     {
     case CPP_PLUS:
       c_parser_consume_token (parser);
-      return PLUS_EXPR;
+      return true;
     case CPP_MINUS:
       c_parser_consume_token (parser);
-      return MINUS_EXPR;
+      return false;
     default:
       gcc_unreachable ();
     }
@@ -6916,11 +6918,10 @@  c_parser_objc_method_type (c_parser *parser)
 static void
 c_parser_objc_method_definition (c_parser *parser)
 {
-  enum tree_code type = c_parser_objc_method_type (parser);
+  bool is_class_method = c_parser_objc_method_type (parser);
   tree decl, attributes = NULL_TREE;
-  objc_set_method_type (type);
   parser->objc_pq_context = true;
-  decl = c_parser_objc_method_decl (parser, &attributes);
+  decl = c_parser_objc_method_decl (parser, is_class_method, &attributes);
   if (decl == error_mark_node)
     return;  /* Bail here. */
 
@@ -6938,7 +6939,7 @@  c_parser_objc_method_definition (c_parser *parser)
     }
 
   parser->objc_pq_context = false;
-  if (objc_start_method_definition (decl, attributes))
+  if (objc_start_method_definition (is_class_method, decl, attributes))
     {
       add_stmt (c_parser_compound_statement (parser));
       objc_finish_method_definition (current_function_decl);
@@ -7024,12 +7025,12 @@  c_parser_objc_methodprotolist (c_parser *parser)
 static void
 c_parser_objc_methodproto (c_parser *parser)
 {
-  enum tree_code type = c_parser_objc_method_type (parser);
+  bool is_class_method = c_parser_objc_method_type (parser);
   tree decl, attributes = NULL_TREE;
-  objc_set_method_type (type);
+
   /* Remember protocol qualifiers in prototypes.  */
   parser->objc_pq_context = true;
-  decl = c_parser_objc_method_decl (parser, &attributes);
+  decl = c_parser_objc_method_decl (parser, is_class_method, &attributes);
   /* Forget protocol qualifiers now.  */
   parser->objc_pq_context = false;
 
@@ -7042,7 +7043,7 @@  c_parser_objc_methodproto (c_parser *parser)
     }
   
   if (decl != error_mark_node)
-    objc_add_method_declaration (decl, attributes);
+    objc_add_method_declaration (is_class_method, decl, attributes);
 
   c_parser_skip_until_found (parser, CPP_SEMICOLON, "expected %<;%>");
 }
@@ -7115,7 +7116,7 @@  c_parser_objc_maybe_method_attributes (c_parser* p
 */
 
 static tree
-c_parser_objc_method_decl (c_parser *parser, tree *attributes)
+c_parser_objc_method_decl (c_parser *parser, bool is_class_method, tree *attributes)
 {
   tree type = NULL_TREE;
   tree sel;
@@ -7206,7 +7207,7 @@  static tree
   if (attr_err)
     return error_mark_node;
 
-  return objc_build_method_signature (type, sel, parms, ellipsis);
+  return objc_build_method_signature (is_class_method, type, sel, parms, ellipsis);
 }
 
 /* Parse an objc-type-name.