-
Notifications
You must be signed in to change notification settings - Fork 54
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
[Java.Base, generator] Bind all of package java.io #968
Conversation
4ed40ed
to
a080535
Compare
Context: bc5bcf4 Bind all classes and interfaces in the `java.io` package. Binding the [`java.io.StringBufferInputStream.buffer`][0] field exposed a `generator` bug: property setters for `String` fields would not compile, producing: partial class StringBufferInputStream { protected string? Buffer { get { ... } set { const string __id = "buffer.Ljava/lang/String;"; var native_value = global::Java.Interop.JniEnvironment.Strings.NewString (value); try { _members.InstanceFields.SetValue (__id, this, native_value?.PeerReference ?? default); } finally { global::Java.Interop.JniObjectReference.Dispose (ref native_value); GC.KeepAlive (value); } } } } Specifically, `native_value?.PeerReference` is not valid, and unnecessary. This was due to a bug in `BoundFieldAsProperty.cs`, as `String` field marshaling hit the "Object" marshaling path, which was not warranted in this case, as `String` is treated specially. Fix this bug. Binding [`java.io.ObjectOutputStream.PutField.write(ObjectOutput)`][2] presented a larger binding problem: it's an `abstract` method on an `abstract` class, *but* the `PutFieldInvoker.Write()` method was `[Obsolete]` when `PutField.Write()` was not! public partial class /* ObjectOutputStream. */ PutField { public abstract void Write (Java.IO.IObjectOutput? p0); } internal partial class /* ObjectOutputStream. */ PutFieldInvoker : PutField { [Obsolete (@"deprecated")] public override void Write (Java.IO.IObjectOutput? p0) => ... } This state of affairs triggers a CS0809 warning, which is an error in Release configuration builds: ##[error]src/Java.Base/obj/Release-net6.0/mcw/Java.IO.ObjectOutputStream.cs(311,32) Error CS0809: Obsolete member 'ObjectOutputStream.PutFieldInvoker.Write(IObjectOutput?)' overrides non-obsolete member 'ObjectOutputStream.PutField.Write(IObjectOutput?)' Turns Out that `BoundMethodAbstractDeclaration.cs` never "forwarded" deprecated attributes? Meaning that `abstract` methods would *never* be `[Obsolete]`, even if their Java peer was `@deprecated`? Update `BoundMethodAbstractDeclaration` so that if the Java method is deprecated, the bound `abstract` method is `[Obsolete]`. This fixes the CS0809, as now `PutField.Write()` and `PutFieldInvoker.Write()` are consistent with each other. TODO: should this change be done for *all* codegen targets? Finally, in order to make it easier to view and review the current `Java.Base.dll` API -- without requiring that it be built locally -- add a `_GenerateRefApi` target to `Java.Base.targets` which uses the `Microsoft.DotNet.GenAPI` NuGet package to generate "reference assembly source" for `Java.Base.dll`. This is stored in `src\Java.Base-ref.cs`. [0]: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/StringBufferInputStream.html#buffer [1]: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/ObjectOutputStream.PutField.html#write(java.io.ObjectOutput)
a080535
to
333c201
Compare
@@ -52,6 +52,11 @@ public BoundMethodAbstractDeclaration (GenBase gen, Method method, CodeGeneratio | |||
if (method.DeclaringType.IsGeneratable) | |||
Comments.Add ($"// Metadata.xml XPath method reference: path=\"{method.GetMetadataXPathReference (method.DeclaringType)}\""); | |||
|
|||
// TODO: shouldn't `[Obsolete]` be added for *all* CodeGenerationTargets? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like it would make sense to always prevent that warning. In the real world it's nearly impossible to bind libraries with <TreatWarningsAsErrors>
, but it's something we should continue working towards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpobst: The implicit question here is whether all CodeGenerationTargets should add [Obsolete]
to abstract methods, or just CodeGenerationTarget.JavaInterop1
.
I opted to only update CodeGenerationTarget.JavaInterop1
for expediency, but I suspect this should be done for everything.
Context: bc5bcf4
Bind all classes and interfaces in the
java.io
package.Binding the
java.io.StringBufferInputStream.buffer
fieldexposed a
generator
bug: property setters forString
fields wouldnot compile, producing:
Specifically,
native_value?.PeerReference
is not valid, andunnecessary. This was due to a bug in
BoundFieldAsProperty.cs
,as
String
field marshaling hit the "Object" marshaling path, whichwas not warranted in this case, as
String
is treated specially.Fix this bug.
Binding [
java.io.ObjectOutputStream.PutField.write(ObjectOutput)
][2]presented a larger binding problem: it's an
abstract
method on anabstract
class, but thePutFieldInvoker.Write()
method was[Obsolete]
whenPutField.Write()
was not!This state of affairs triggers a CS0809 warning, which is an error in
Release configuration builds:
Turns Out that
BoundMethodAbstractDeclaration.cs
never "forwarded"deprecated attributes? Meaning that
abstract
methods would neverbe
[Obsolete]
, even if their Java peer was@deprecated
?Update
BoundMethodAbstractDeclaration
so that if the Java method isdeprecated, the bound
abstract
method is[Obsolete]
. This fixesthe CS0809, as now
PutField.Write()
andPutFieldInvoker.Write()
are consistent with each other.
TODO: should this change be done for all codegen targets?
Finally, in order to make it easier to view and review the
current
Java.Base.dll
API -- without requiring that it be builtlocally -- add a
_GenerateRefApi
target toJava.Base.targets
which uses the
Microsoft.DotNet.GenAPI
NuGet package to generate"reference assembly source" for
Java.Base.dll
. This is stored insrc\Java.Base-ref.cs
.