Skip to content

Generator do not expose nonpublic #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 14, 2016

Conversation

atsushieno
Copy link
Contributor

No description provided.

…adjuster.

api-24.xml.in was already generated with the almost latest tools,
but it has some new members probably due to android.jar updates.

Others were generated by old tools so they had extraneous generic
constraints.
…ster.

There were non-breaking API definition files changes caused by
dotnet/java-interop#94

This update reflects the changes.
@atsushieno atsushieno force-pushed the generator-do-not-expose-nonpublic branch from 7ccc864 to 5847c2d Compare October 12, 2016 09:56
@jonpryor
Copy link
Member

Aside: it would be nice if GitHub would actually show the changes. Fortunately, there's the .patch URL.

Many of the changes are due to:

-        <typeParameter name="V">
-          <genericConstraints>
-            <genericConstraint type="java.lang.Object">
-            </genericConstraint>
-          </genericConstraints>
-        </typeParameter>
+        <typeParameter name="V"></typeParameter>

which seems odd, but should be meaningless.

@jonpryor
Copy link
Member

Should this include a corresponding Java.Interop bump? It wouldn't be strictly required, since we're not using class-parse/etc. to generate these files within a normal build, but it might be nice in a "the contents of this PR correspond to this Java.Interop commit..." sense.

@atsushieno
Copy link
Contributor Author

The genericConstraints changes were from older ApiXmlAdjuster fix (they were different from jar2xml outputs which never generated those extraneous elements), not by that Java.Interop changeset.

@jonpryor jonpryor merged commit d481b4d into dotnet:master Oct 14, 2016
radical pushed a commit that referenced this pull request May 8, 2018
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;}
	}
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants