-
Notifications
You must be signed in to change notification settings - Fork 63
[Java.Base, generator] Bind all of package java.io #968
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
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
| 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.iopackage.Binding the
java.io.StringBufferInputStream.bufferfieldexposed a
generatorbug: property setters forStringfields wouldnot compile, producing:
Specifically,
native_value?.PeerReferenceis not valid, andunnecessary. This was due to a bug in
BoundFieldAsProperty.cs,as
Stringfield marshaling hit the "Object" marshaling path, whichwas not warranted in this case, as
Stringis treated specially.Fix this bug.
Binding [
java.io.ObjectOutputStream.PutField.write(ObjectOutput)][2]presented a larger binding problem: it's an
abstractmethod on anabstractclass, 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.csnever "forwarded"deprecated attributes? Meaning that
abstractmethods would neverbe
[Obsolete], even if their Java peer was@deprecated?Update
BoundMethodAbstractDeclarationso that if the Java method isdeprecated, the bound
abstractmethod 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.dllAPI -- without requiring that it be builtlocally -- add a
_GenerateRefApitarget toJava.Base.targetswhich uses the
Microsoft.DotNet.GenAPINuGet package to generate"reference assembly source" for
Java.Base.dll. This is stored insrc\Java.Base-ref.cs.