Skip to content

[generator] Ensure non-constant static interface fields are generated as interface properties. #831

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 1 commit into from
May 5, 2021

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented May 4, 2021

Fixes: #821
Context: dotnet/android#5818
Context: #459
Context: #509

When we began generating static methods and properties on interfaces when running with DIM enabled, it looks like we did not include static fields that cannot be treated as constants.

Thus, given the following example from dotnet/android#5818:

public interface EGL10 {
  public static final int EGL_ALPHA_FORMAT = 12424;
  public static final EGLSurface EGL_NO_SURFACE;
}

we only generate:

public partial interface IEGL10 {
  public const int EglAlphaFormat = (int) 12424;
}

This is an issue because we [Obsolete]'d the version in the *class* EGL10 pointing to the non-existent interface version:

[Obsolete ("Use 'Javax.Microedition.Khronos.Egl.IEGL10.EglNoSurface'. This class will be removed in a future release.")]
public static Javax.Microedition.Khronos.Egl.EGLSurface? EglNoSurface {
  ...
}

This commit generates the static interfaces fields as properties when -lang-features=default-interface-methods is specified.

@jpobst jpobst force-pushed the interface-field-properties branch from 7f745de to 4ed718c Compare May 4, 2021 19:10
@jpobst jpobst marked this pull request as ready for review May 4, 2021 19:22

// Metadata.xml XPath interface reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']"
[Register ("com/xamarin/android/MyInterface", "", "Com.Xamarin.Android.IMyInterfaceInvoker")]
public partial interface IMyInterface : IJavaObject, IJavaPeerable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this interface declaration also contain:

public const int EglNativeVisualId = (int) 12334;

I would expect the IMyInterface type to "mirror"/contain the same members as the MyInterface type, yet IMyInterface.EglNativeVisualId doesn't exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interface constants are gated behind -lang-features=interface-constants, which isn't enabled in this test suite.

I went back and forth on this and decided that "static interface fields" falls more under default-interface-methods than interface-constants. DIM is poorly named and really means "default AND static interface members" in this context.

@jonpryor jonpryor merged commit 12e670a into main May 5, 2021
@jonpryor jonpryor deleted the interface-field-properties branch May 5, 2021 23:08
@jpobst jpobst added this to the 11.4 (16.11 / 8.11) milestone May 6, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 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.

Generate static interface fields when DIM is enabled
2 participants