[m-rev.] For review: Java implementation of array library

[Fergus Henderson]

On 08-Jan-2004, James Goddard <goddardjames at yahoo.com> wrote:
> Implement some library procedures for the Java back end.
> 
> +:- pragma foreign_proc("Java",
> +	array__resize(Array0::array_di, Size::in, Item::in,
> +		Array::array_uo),
> +	[will_not_call_mercury, promise_pure, thread_safe],
> +"
> +	java.lang.Class itemClass = Item.getClass();
> +
> +	if (Array0 == null) {
> +		Array = java.lang.reflect.Array.newInstance(itemClass, Size);
> +		for (int i = 0; i < Size; i++) {
> +			java.lang.reflect.Array.set(Array, i, Item);
> +		}

I notice here that if Size == 0, this will create a non-null Array
with length zero.  That means there are two different representations
for empty arrays, which might cause problems...

> +	} else if (java.lang.reflect.Array.getLength(Array0) == Size) {
> +		Array = Array0;
> +	} else if (java.lang.reflect.Array.getLength(Array0) > Size) {
> +		Array = java.lang.reflect.Array.newInstance(itemClass, Size);
> +		for (int i = 0; i < Size; i++) {
> +			java.lang.reflect.Array.set(Array, i,
> +					java.lang.reflect.Array.get(Array0, i)
> +					);
> +		}
> +	} else {
> +		Array = java.lang.reflect.Array.newInstance(itemClass, Size);
> +
> +		int i;
> +		for (i = 0; i < java.lang.reflect.Array.getLength(Array0); i++)
> +		{
> +			java.lang.reflect.Array.set(Array, i,
> +					java.lang.reflect.Array.get(Array0, i)
> +					);
> +		}
> +		for (/*i = Array0.length*/; i < Size; i++) {
> +			java.lang.reflect.Array.set(Array, i, Item);
> +		}
> +	}
> +").

The second-last case here, "getLength(Array0) > Size", is unnecessary --
the code in the final "else" case will do the right thing for the case
where getLength(Array0) > Size, won't it?

>  %-----------------------------------------------------------------------------%
>  
>  :- pragma foreign_decl("C", "
> @@ -938,6 +1081,24 @@
>  	System.Array.Copy(Array0, Array, Size);
>  ").
>  
> +:- pragma foreign_proc("Java",
> +	array__shrink_2(Array0::array_di, Size::in, Array::array_uo),
> +	[will_not_call_mercury, promise_pure, thread_safe],
> +"
> +	if (Array0 == null) {
> +		Array = null;
> +	} else {
> +		java.lang.Class itemClass = java.lang.reflect.Array.
> +				get(Array0, 0).getClass();

This won't work if Array0 is a non-null array with length zero,
as would be created by your code for array__resize -- see my first
comment above.

Probably the best fix is to change array__resize.


Apart from the issues mentioned above, this patch looks fine.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------