Skip to content

Commit 449e5de

Browse files
radekdoulikjonpryor
authored andcommitted
[Java.Interop] Use ref return for elements access (#260)
While reviewing [DoNotThrowInUnexpectedLocationRule][throw] regarding `JniArrayElements.Elements` we discussed the issue and came up with a slight semantic API overhaul. [throw]: https://github.com/spouliot/gendarme/wiki/Gendarme.Rules.Exceptions.DoNotThrowInUnexpectedLocationRule(2.10) Previously, we had: public abstract partial class JniArrayElements : IDisposable { public IntPtr Elements {get;} public void CopyToJava(); public void Dispose(); public void Release(JniReleaseArrayElementsMode releaseMode); } public sealed class JniInt32ArrayElements : JniArrayElements { public new unsafe int* Elements {get;} } The *idea* of `JniArrayElements` was that it refers to a contiguous blob of memory, starting at address `JniArrayElements.Elements`. The thinking was that `JniArrayElements` would allow reading or otherwise manipulating the memory regions without needing to know the underlying element type. On further discussion, this abstraction was missing something important: the *size* of the region of memory that `JniArrayElements.Elements` refers to. Additionally, with the advent of [C# 7 ref returns][rr], we thought that providing a ref-return indexer would be preferable to using a shadowing/re-declared `Elements` property. [rr]: https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/ref-returns We also considered using `Span<T>/Memory<T>`; it is not available in mono at this time. It should be possible to add support for them when they are supported. With these changes, the updated public API is: public abstract partial class JniArrayElements : IDisposable { public IntPtr Elements {get;} public int Size {get;} public void CopyToJava(); public void Dispose(); public void Release(JniReleaseArrayElementsMode releaseMode); } public sealed class JniInt32ArrayElements : JniArrayElements { public ref int this [int index] {get;} }
1 parent 59e7d8a commit 449e5de

File tree

5 files changed

+187
-75
lines changed

5 files changed

+187
-75
lines changed

gendarme-ignore.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,3 +688,15 @@ M: System.String <>__AnonType1`2::ToString()
688688
R: Gendarme.Rules.Exceptions.DoNotThrowInUnexpectedLocationRule
689689
# throwing ArgumentOutOfRangeException is OK according to the documentation https://msdn.microsoft.com/library/system.collections.ilist.item(v=vs.110).aspx
690690
M: T Java.Interop.JavaObjectArray`1::get_Item(System.Int32)
691+
# we want these to check for disposed state, throwing ObjectDisposedException is better and not much different than suggested InvalidOperationException
692+
M: System.Boolean& Java.Interop.JniBooleanArrayElements::get_Item(System.Int32)
693+
M: System.SByte& Java.Interop.JniSByteArrayElements::get_Item(System.Int32)
694+
M: System.Char& Java.Interop.JniCharArrayElements::get_Item(System.Int32)
695+
M: System.Int16& Java.Interop.JniInt16ArrayElements::get_Item(System.Int32)
696+
M: System.Int32& Java.Interop.JniInt32ArrayElements::get_Item(System.Int32)
697+
M: System.Int64& Java.Interop.JniInt64ArrayElements::get_Item(System.Int32)
698+
M: System.Single& Java.Interop.JniSingleArrayElements::get_Item(System.Int32)
699+
M: System.Double& Java.Interop.JniDoubleArrayElements::get_Item(System.Int32)
700+
# we want to check for disposed state here
701+
M: System.IntPtr Java.Interop.JniArrayElements::get_Elements()
702+
M: System.Int32 Java.Interop.JniArrayElements::get_Size()

src/Java.Interop/Java.Interop/JavaArray.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,15 +295,17 @@ void IList<T>.RemoveAt (int index)
295295
public abstract class JniArrayElements : IDisposable {
296296

297297
IntPtr elements;
298+
int size;
298299

299-
internal JniArrayElements (IntPtr elements)
300+
internal JniArrayElements (IntPtr elements, int size)
300301
{
301302
if (elements == IntPtr.Zero)
302303
throw new ArgumentException ("'elements' must not be IntPtr.Zero.", nameof (elements));
303304
this.elements = elements;
305+
this.size = size;
304306
}
305307

306-
bool IsDisposed {
308+
internal bool IsDisposed {
307309
get {return elements == IntPtr.Zero;}
308310
}
309311

@@ -315,6 +317,14 @@ public IntPtr Elements {
315317
}
316318
}
317319

320+
public int Size {
321+
get {
322+
if (IsDisposed)
323+
throw new ObjectDisposedException (GetType ().FullName);
324+
return size;
325+
}
326+
}
327+
318328
protected abstract void Synchronize (JniReleaseArrayElementsMode releaseMode);
319329

320330
public void CopyToJava ()

0 commit comments

Comments
 (0)