diff mbox

fix -Wsign-compare error in objc-act.c (PR objc/50743)

Message ID 20122.49140.381473.924718@pilspetsen.it.uu.se
State New
Headers show

Commit Message

Mikael Pettersson Oct. 16, 2011, 11:28 a.m. UTC 
This fixes a bootstrap failure on trunk with objc enabled, caused by a
recent change which introduced for-loops that compare a size_t index
variable with a TREE_VEC_LENGTH limit.  TREE_VEC_LENGTH returns a signed
integer, resulting in a signed/unsigned comparison.  The C++ front-end
fails to catch this error due to PR50012, but the C front-end correctly
catches it, breaking --disable-build-poststage1-with-cxx bootstraps.

Fixed by casting TREE_VEC_LENGTH to size_t before the comparisons.

Nicola Pero said the patch looked good, but also said he couldn't formally
approve it.

Ok for trunk?

(I don't have svn write access so I'll need someone else to commit it if
it's approved.)

/Mikael

gcc/objc/

2011-10-16  Mikael Pettersson  <mikpe@it.uu.se>

	PR objc/50743
	* objc-act.c (check_duplicates): Cast TREE_VEC_LENGTH result
	to size_t to avoid signed/unsigned comparison.
	(insert_method_into_method_map): Likewise.

Comments

Mike Stump Oct. 17, 2011, 1:35 a.m. UTC | #1 
On Oct 16, 2011, at 4:28 AM, Mikael Pettersson wrote:
> This fixes a bootstrap failure on trunk with objc enabled

> Ok for trunk?

Ok.
Nicola Pero Oct. 17, 2011, 8:52 a.m. UTC | #2 
> (I don't have svn write access so I'll need someone else to commit it if
> it's approved.)

I can apply it for you.  But ... do you have a copyright assignment in place
for contributions to GCC ?  The patch looks small and trivial enough that
I think I can apply it without a signed copyright assignment form, but if you
plan on contributing more, it would make sense to sign one. :-)

Thanks
Mikael Pettersson Oct. 17, 2011, 10:19 p.m. UTC | #3 
Nicola Pero writes:
 > > (I don't have svn write access so I'll need someone else to commit it if
 > > it's approved.)
 > 
 > I can apply it for you.  But ... do you have a copyright assignment in place
 > for contributions to GCC ?  The patch looks small and trivial enough that
 > I think I can apply it without a signed copyright assignment form, but if you
 > plan on contributing more, it would make sense to sign one. :-)

The FSF has had my papers since Feb '11, but are stalling trying to decide
whether to accept my employer's slightly non-standard disclaimer or not.

I'd rather not spam gcc-patches by attaching the GPLv3 COPYING to every
patch submission, but I can do so if necessary.  (E.g., a copy of the patch
inline for reviewability, an attachment with a compressed tarball containing
the patch, COPYING, and a README linking the two.)
Nicola Pero Oct. 18, 2011, 11:34 a.m. UTC | #4 
> The FSF has had my papers since Feb '11, but are stalling trying to decide
> whether to accept my employer's slightly non-standard disclaimer or not.

Ah.  That seems a long time even for this kind of things.

Anyhow, this particular patch consisted of exactly 4 casts, so it seems to fall in the case
of a "tiny patch" which doesn't require copyright assignment, so I applied it for you.

Thanks
diff mbox

Patch

--- gcc-4.7-20111015/gcc/objc/objc-act.c.~1~	2011-10-14 12:19:01.000000000 +0200
+++ gcc-4.7-20111015/gcc/objc/objc-act.c	2011-10-16 11:53:56.000000000 +0200
@@ -5070,7 +5070,7 @@  check_duplicates (tree method, int metho
      alignment.  */
   if (!warn_strict_selector_match)
     {
-      for (i = 0; i < TREE_VEC_LENGTH (method); i++)
+      for (i = 0; i < (size_t) TREE_VEC_LENGTH (method); i++)
 	if (!comp_proto_with_proto (first_method, TREE_VEC_ELT (method, i), 0))
 	  goto issue_warning;
       
@@ -5103,7 +5103,7 @@  check_duplicates (tree method, int metho
 	      identifier_to_locale (gen_method_decl (first_method)));
     }
   
-  for (i = 0; i < TREE_VEC_LENGTH (method); i++)
+  for (i = 0; i < (size_t) TREE_VEC_LENGTH (method); i++)
     {
       bool type = TREE_CODE (TREE_VEC_ELT (method, i)) == INSTANCE_METHOD_DECL;
       
@@ -5825,7 +5825,7 @@  insert_method_into_method_map (bool clas
 	  /* Check all the existing prototypes.  If any matches the
 	     one we need to add, there is nothing to do because it's
 	     already there.  */
-	  for (i = 0; i < TREE_VEC_LENGTH (existing_entry); i++)
+	  for (i = 0; i < (size_t) TREE_VEC_LENGTH (existing_entry); i++)
 	    if (comp_proto_with_proto (method, TREE_VEC_ELT (existing_entry, i), 1))
 	      return;
 
@@ -5837,7 +5837,7 @@  insert_method_into_method_map (bool clas
 	  new_entry = make_tree_vec (TREE_VEC_LENGTH (existing_entry) + 1);
 	  
 	  /* Copy the methods from the existing vector.  */
-	  for (i = 0; i < TREE_VEC_LENGTH (existing_entry); i++)
+	  for (i = 0; i < (size_t) TREE_VEC_LENGTH (existing_entry); i++)
 	    TREE_VEC_ELT (new_entry, i) = TREE_VEC_ELT (existing_entry, i);
 	  
 	  /* Add the new method at the end.  */