Patchwork Fix for objc/45925

login
register
mail settings
Submitter Nicola Pero
Date Oct. 7, 2010, 9:07 a.m.
Message ID <1286442447.74294365@192.168.2.230>
Download mbox | patch
Permalink /patch/67012/
State New
Headers show

Comments

Nicola Pero - Oct. 7, 2010, 9:07 a.m.
This patch fixes PR objc/45925.  A missing cast in the new fast enumeration code.

Ok to apply ?

Thanks
Rainer Orth - Oct. 7, 2010, 9:12 a.m.
"Nicola Pero" <nicola.pero@meta-innovation.com> writes:

> Index: objc-act.c
> ===================================================================
> --- objc-act.c  (revision 165067)
> +++ objc-act.c  (working copy)
> @@ -10325,7 +10325,7 @@ objc_finish_foreach_loop (location_t location, tre
>                                     (NULL_TREE, build_int_cst (NULL_TREE, 16), NULL_TREE))));
>    }
>  #endif
> -  t = build2 (MODIFY_EXPR, void_type_node, objc_foreach_batchsize_decl, t);
> +  t = build2 (MODIFY_EXPR, void_type_node, objc_foreach_batchsize_decl, convert (long_unsigned_type_node, t));

Please break this line (again below).

Thanks.
        Rainer
IainS - Oct. 7, 2010, 9:17 a.m.
On 7 Oct 2010, at 10:07, Nicola Pero wrote:

> This patch fixes PR objc/45925.  A missing cast in the new fast  
> enumeration code.

should we also have a diagnostic for the (presumed) user error?
[that could be a TODO - not to delay solving the ICE].
Iain

> Ok to apply ?
>
> Thanks
>
> Index: objc-act.c
> ===================================================================
> --- objc-act.c  (revision 165067)
> +++ objc-act.c  (working copy)
> @@ -10325,7 +10325,7 @@ objc_finish_foreach_loop (location_t  
> location, tre
>                                    (NULL_TREE, build_int_cst  
> (NULL_TREE, 16), NULL_TREE))));
>   }
> #endif
> -  t = build2 (MODIFY_EXPR, void_type_node,  
> objc_foreach_batchsize_decl, t);
> +  t = build2 (MODIFY_EXPR, void_type_node,  
> objc_foreach_batchsize_decl, convert (long_unsigned_type_node, t));
>   SET_EXPR_LOCATION (t, location);
>   append_to_statement_list (t, &BIND_EXPR_BODY (bind));
>
> @@ -10498,7 +10498,7 @@ objc_finish_foreach_loop (location_t  
> location, tre
>                                    (NULL_TREE, build_int_cst  
> (NULL_TREE, 16), NULL_TREE))));
>   }
> #endif
> -  t = build2 (MODIFY_EXPR, void_type_node,  
> objc_foreach_batchsize_decl, t);
> +  t = build2 (MODIFY_EXPR, void_type_node,  
> objc_foreach_batchsize_decl, convert (long_unsigned_type_node, t));
>   SET_EXPR_LOCATION (t, location);
>   append_to_statement_list (t, &BIND_EXPR_BODY (next_batch_bind));
>
> Index: ChangeLog
> ===================================================================
> --- ChangeLog   (revision 165067)
> +++ ChangeLog   (working copy)
> @@ -1,3 +1,9 @@
> +2010-10-07  Nicola Pero  <nicola.pero@meta-innovation.com>
> +
> +       PR objc/45925
> +       * objc-act.c (objc_finish_foreach_loop): Convert return  
> value of
> +       countByEnumeratingWithState:objects:count: to long unsigned  
> int.
> +
> 2010-10-06  Nicola Pero  <nicola.pero@meta-innovation.com>
>
>        * README: Obsolete file removed.
>
>
Nicola Pero - Oct. 7, 2010, 9:23 a.m.
Thanks Rainer

I will before committing :-)

Thanks

-----Original Message-----
From: "Rainer Orth" <ro@CeBiTec.Uni-Bielefeld.DE>
Sent: Thursday, 7 October, 2010 11:12
To: "Nicola Pero" <nicola.pero@meta-innovation.com>
Cc: "gcc-patches@gnu.org" <gcc-patches@gnu.org>
Subject: Re: Fix for objc/45925

"Nicola Pero" <nicola.pero@meta-innovation.com> writes:

> Index: objc-act.c
> ===================================================================
> --- objc-act.c  (revision 165067)
> +++ objc-act.c  (working copy)
> @@ -10325,7 +10325,7 @@ objc_finish_foreach_loop (location_t location, tre
>                                     (NULL_TREE, build_int_cst (NULL_TREE, 16), NULL_TREE))));
>    }
>  #endif
> -  t = build2 (MODIFY_EXPR, void_type_node, objc_foreach_batchsize_decl, t);
> +  t = build2 (MODIFY_EXPR, void_type_node, objc_foreach_batchsize_decl, convert (long_unsigned_type_node, t));

Please break this line (again below).

Thanks.
        Rainer
Nicola Pero - Oct. 7, 2010, 9:39 a.m.
>> This patch fixes PR objc/45925.  A missing cast in the new fast  
>> enumeration code.
>
> should we also have a diagnostic for the (presumed) user error?
> [that could be a TODO - not to delay solving the ICE].

Having look into this to produce the bugfix, I'm no longer sure it is a user error. :-)

I'd say it was a bug in the new fast enumeration code - a missing cast. ;-)

We probably don't even want to change the testcases.

It would be a user error if the method signature was declared to be something 
completely different (eg, returning an object) and while it would be good 
to double-check for that (and we could add a new bugzilla issue about it), it is 
not terribly relevant to actual real-world usage. ;-)

The most likely problem is that the integer types are declared differently 
by the user.  And, the Apple API seems to specify that 'unsigned int' 
is used as return type in 32-bit, and 'long unsigned int' is used in 
64-bit; and the same for the 'len' parameter.  So, in a sense, they already do
it and most likely GNUstep will do the same as it's implementating the same API ;-)

But (once we fix objc/45924 and add the missing cast) there is nothing 
to change there in the compiler to support that (other than our documentation).

You can use different integer types in your method declaration/implementation, 
and it should still work.

The 'len' argument always receives the constant integer value 16 as parameter, 
which should fit into any integer type, no matter what integer type you declare.

The return value will be whatever type you declare, and is then cast to 
a 'long unsigned int' which hopefully is big enough (also considering that at runtime
the return value will always be a number between 0 and 16 since it has to be smaller
than 'len' according to the specification).

Tonight I'll submit a patch that updates the documentation to clarify this issue. :-)

Thanks

PS: Let me know if I missed something
Mike Stump - Oct. 7, 2010, 2:54 p.m.
On Oct 7, 2010, at 2:07 AM, "Nicola Pero" <nicola.pero@meta-innovation.com> wrote:
> This patch fixes PR objc/45925.  A missing cast in the new fast enumeration code.
> 
> Ok to apply ?

Ok.
>

Patch

Index: objc-act.c
===================================================================
--- objc-act.c  (revision 165067)
+++ objc-act.c  (working copy)
@@ -10325,7 +10325,7 @@  objc_finish_foreach_loop (location_t location, tre
                                    (NULL_TREE, build_int_cst (NULL_TREE, 16), NULL_TREE))));
   }
 #endif
-  t = build2 (MODIFY_EXPR, void_type_node, objc_foreach_batchsize_decl, t);
+  t = build2 (MODIFY_EXPR, void_type_node, objc_foreach_batchsize_decl, convert (long_unsigned_type_node, t));
   SET_EXPR_LOCATION (t, location);
   append_to_statement_list (t, &BIND_EXPR_BODY (bind));
 
@@ -10498,7 +10498,7 @@  objc_finish_foreach_loop (location_t location, tre
                                    (NULL_TREE, build_int_cst (NULL_TREE, 16), NULL_TREE))));
   }
 #endif
-  t = build2 (MODIFY_EXPR, void_type_node, objc_foreach_batchsize_decl, t);
+  t = build2 (MODIFY_EXPR, void_type_node, objc_foreach_batchsize_decl, convert (long_unsigned_type_node, t));
   SET_EXPR_LOCATION (t, location);
   append_to_statement_list (t, &BIND_EXPR_BODY (next_batch_bind));
 
Index: ChangeLog
===================================================================
--- ChangeLog   (revision 165067)
+++ ChangeLog   (working copy)
@@ -1,3 +1,9 @@ 
+2010-10-07  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR objc/45925
+       * objc-act.c (objc_finish_foreach_loop): Convert return value of
+       countByEnumeratingWithState:objects:count: to long unsigned int.
+
 2010-10-06  Nicola Pero  <nicola.pero@meta-innovation.com>
 
        * README: Obsolete file removed.