-
Notifications
You must be signed in to change notification settings - Fork 64
[generator] Use DIM to support static interface methods. #487
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
.../Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteStaticInterfaceProperty.txt
Outdated
Show resolved
Hide resolved
.../Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteStaticInterfaceProperty.txt
Show resolved
Hide resolved
|
|
||
| [Register ("java/code/IMyInterface", DoNotGenerateAcw=true)] | ||
| [global::System.Obsolete ("Use the 'MyInterface' type. This type will be removed in a future release.")] | ||
| public abstract class MyInterfaceConsts : MyInterface { |
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.
Why are we emitting an empty MyInterfaceConsts type? If the interface had some constants, that would be one thing, but it doesn't...
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.
The MyInterfaceConsts type is always generated if we generate the MyInterface class. It does not attempt to detect why we are writing a MyInterface class.
Logic for determining if we are writing a MyInterface class:
interface.Fields.Any () || interface.Methods.Where (m => m.IsStatic)
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.
Should we alter that logic? Particularly considering that the generated type is [Obsolete] (lol?), and there's no reason for the type to exist in a C#8 default-interface-members world order?
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.
This is technically a break if an existing library enables DIM, so we discussed making these [Obsolete(error:true)] so maybe eventually we could remove them. (For a separate PR.)
.../Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteStaticInterfaceMethod.txt
Show resolved
Hide resolved
ddbdfae to
d6b7833
Compare
d6b7833 to
fe4deb0
Compare
Using the new Default Interface Members support (29f9707), and `generator --lang-features=default-interface-methods`, we can now also support static interface methods/properties. This Java interface: // Java public interface StaticMethodsInterface { static int foo () { return 10; } static int getValue () { return 3; } static void setValue (int value) { } } can now be bound as: // C# Binding public partial interface IStaticMethodsInterface { static new readonly JniPeerMembers _members = new JniPeerMembers ( "java/code/StaticMethodsInterface", typeof (IStaticMethodsInterface), isInterface: true); public static unsafe int Foo () { return _members.StaticMethods.InvokeInt32Method ("foo.()I", null); } public static unsafe int Value { get { return _members.StaticMethods.InvokeInt32Method ("getValue.()I", null); } set { JniArgumentValue* __args = stackalloc JniArgumentValue [1]; __args [0] = new JniArgumentValue (value); _members.StaticMethods.InvokeVoidMethod ("setValue.(I)V", __args); } } } Which can be invoked as: [Test] public void TestStaticMethods () { Assert.AreEqual (10, IStaticMethodsInterface.Foo ()); Assert.AreEqual (3, IStaticMethodsInterface.Value); Assert.DoesNotThrow (() => IStaticMethodsInterface.Value = 5); }
Using the new DIM support we can now also support static interface methods/properties.
Example:
Can now be invoked: