Fix for objc/45925

Submitted by Nicola Pero on Oct. 7, 2010, 9:07 a.m.

Details

Message ID 1286442447.74294365@192.168.2.230
State New
Headers show

Commit Message

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

Comments

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

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.