Patchwork ObjC: remove spurious warnings with -Wall and fast enumeration loops

login
register
mail settings
Submitter Nicola Pero
Date Dec. 6, 2010, 12:33 a.m.
Message ID <1291595616.019710631@192.168.4.58>
Download mbox | patch
Permalink /patch/74311/
State New
Headers show

Comments

Nicola Pero - Dec. 6, 2010, 12:33 a.m.
This patch fixes spurious warnings generated by GCC when an
Objective-C fast enumeration loop is compiled using -Wall.

A number of very annoying warnings ("statement with no effect") were
always generated for all fast enumeration loops, and had to do with
the fact that we would create a statement out of both the iterating
and collection expressions.  The change in c-parser.c in this patch
fixes this.

Some more warnings were generated when the iterating variable wasn't
used in the loop, as in the example

 unsigned count = 0;

 for (id object in collection)
  count++;

where the compiler would produce a warning that 'object' is set but
not used.  That is technically correct, but not very useful since you
have to supply some iterating variable to produce a fast enumeration
loop, and it seems to be envisaged in the documentation that you may
decide to ignore the iterating variable in the loop (eg, the code
above could be used to count the number of objects in the collection).

Other compilers (Apple's GCC and clang) do not produce a warning in
that case, so in this patch I disabled it for our Objective-C compiler
as well.

A testcase (which this patch fixes) is included.

Ok to commit ?

Thanks
Mike Stump - Dec. 6, 2010, 8:34 p.m.
On Dec 5, 2010, at 4:33 PM, Nicola Pero wrote:
> This patch fixes spurious warnings generated by GCC when an
> Objective-C fast enumeration loop is compiled using -Wall.

> Ok to commit ?

Ok.

Patch

Index: objc/objc-act.c
===================================================================
--- objc/objc-act.c     (revision 167471)
+++ objc/objc-act.c     (working copy)
@@ -13289,6 +13289,18 @@  objc_finish_foreach_loop (location_t loc
   /* type object; */
   /* Done by c-parser.c.  */
 
+  /* Disable warnings that 'object' is unused.  For example the code
+
+     for (id object in collection)
+       i++;
+
+     which can be used to count how many objects there are in the
+     collection is fine and should generate no warnings even if
+     'object' is technically unused.  */
+  TREE_USED (object_expression) = 1;
+  if (DECL_P (object_expression))
+    DECL_READ_P (object_expression) = 1;
+
   /*  id __objc_foreach_collection */
   objc_foreach_collection_decl = objc_create_temporary_var (objc_object_type, "__objc_foreach_collection");
 
Index: objc/ChangeLog
===================================================================
--- objc/ChangeLog      (revision 167471)
+++ objc/ChangeLog      (working copy)
@@ -1,3 +1,8 @@ 
+2010-12-05  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc-act.c (objc_finish_foreach_loop): Mark the
+       object_expression as used.
+
 2010-12-02  Joseph Myers  <joseph@codesourcery.com>
 
        * lang-specs.h: Don't handle -ftraditional.
Index: ChangeLog
===================================================================
--- ChangeLog   (revision 167471)
+++ ChangeLog   (working copy)
@@ -1,3 +1,9 @@ 
+2010-12-05  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * c-parser.c (c_parser_for_statement): Use c_fully_fold() instead
+       of c_process_expr_stmt() for the iterating and collection
+       expressions of an Objective-C fast enumeration loop.
+
 2010-12-03  Jan Hubicka  <jh@suse.cz>
 
        * lto-streamer-in.c (input_cfg): Fix pasto.
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 167471)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,7 @@ 
+2010-12-05  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc.dg/foreach-8.m: New.
+
 2010-12-05  Daniel Kraft  <d@domob.eu>
 
        PR fortran/46794
Index: testsuite/objc.dg/foreach-8.m
===================================================================
--- testsuite/objc.dg/foreach-8.m       (revision 0)
+++ testsuite/objc.dg/foreach-8.m       (revision 0)
@@ -0,0 +1,51 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, December 2010.  */
+/* { dg-options "-Wall" } */
+/* { dg-do compile } */
+
+/* Test that fast enumeration loops where the iterating variable is declared
+   but not used do not generate warnings.  */
+
+/*
+struct __objcFastEnumerationState
+{
+  unsigned long state;
+  id            *itemsPtr;
+  unsigned long *mutationsPtr;
+  unsigned long extra[5];
+};
+*/
+@interface Object
+{
+  Class isa;
+}
+- (unsigned long)countByEnumeratingWithState: (struct __objcFastEnumerationState *)state
+                                     objects:(id *)stackbuf 
+                                       count:(unsigned int)len;
+- (id) enumerator;
+- (Class) classEnumerator;
+@end
+
+unsigned int count_objects_in_collection (id collection)
+{
+  unsigned int count = 0;
+
+  /* The following line should generate no warnings even with
+     -Wall.  */
+  for (id object in collection)
+    count++;
+
+  return count;
+}
+
+unsigned int count_objects_in_collection_2 (id collection)
+{
+  unsigned int count = 0;
+  id object;
+
+  /* The following line should generate no warnings even with
+     -Wall.  */
+  for (object in collection)
+    count++;
+
+  return count;
+}
Index: c-parser.c
===================================================================
--- c-parser.c  (revision 167471)
+++ c-parser.c  (working copy)
@@ -4811,8 +4811,7 @@  c_parser_for_statement (c_parser *parser
                is_foreach_statement = true;
                if (! lvalue_p (init_expression))
                  c_parser_error (parser, "invalid iterating variable in fast enumeration");
-               object_expression = c_process_expr_stmt (loc, init_expression);
-
+               object_expression = c_fully_fold (init_expression, false, NULL);
              }
            else
              {
@@ -4853,7 +4852,8 @@  c_parser_for_statement (c_parser *parser
       else
        {
          if (is_foreach_statement)
-           collection_expression = c_process_expr_stmt (loc, c_parser_expression (parser).value);
+           collection_expression = c_fully_fold (c_parser_expression (parser).value,
+                                                 false, NULL);
          else
            incr = c_process_expr_stmt (loc, c_parser_expression (parser).value);
        }