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

Submitted by Nicola Pero on Dec. 6, 2010, 12:33 a.m.

Details

Message ID 1291595616.019710631@192.168.4.58
State New
Headers show

Commit Message

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

Comments

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 hide | download patch | download mbox

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);
        }