Skip to content

Conversation

@atsushieno
Copy link
Contributor

Until now, ApiXmlAdjuster was trying to resolve types using
ClassGen.BaseGen property which is in fact assigned only after Validate()
step in generator. API XML analyzer work is done way before that step,
and therefore it failed to resolve types from managed code (DLL references).

Missing type hierarchy in Java type model from managed code means that
any bindings that references Mono.Android (which wouldn't?) had to rely
on insufficient hierarchy. Especially, it failed to resolve method
overrides when the target Java library types overrode methods in android.jar
indirectly.

SherlockExpandableListActivity.addContentView() was such a method.

And it had appeared as a regression from our MSBuild tests[*1].

Fixes:

  • removed dependency on BaseGen in ApiXmlAdjuster. Use BaseType instead,
    which doesn't depend on validation step.
    • Though this change uncovered an issue that Ctor and Parameter needed
      to hold Java type information. So, made some changes to them.
  • Now that ApiXmlAdjuster is getting valid managed type hierarchy, it
    had uncovered another kind of issues: managed objects need to exist
    to not cause some null crashes. Hence there is ManagedType class now.
    • Type resolution now premises this ManagedType in type resolver code.
  • Fixed incorrect Load() invocation for ClassGen and InterfaceGen (that
    had caused missing BaseType information).
  • Added (almost irrelevant) ApiXmlAdjuster test to make sure override
    resolution is done for overrides from indirect inheritance.

[*1] that needs to be OSS-ed...

…ster.

Until now, ApiXmlAdjuster was trying to resolve types using
ClassGen.BaseGen property which is in fact assigned only after Validate()
step in generator. API XML analyzer work is done way before that step,
and therefore it failed to resolve types from managed code (DLL references).

Missing type hierarchy in Java type model from managed code means that
any bindings that references Mono.Android (which wouldn't?) had to rely
on insufficient hierarchy. Especially, it failed to resolve method
overrides when the target Java library types overrode methods in android.jar
indirectly.

SherlockExpandableListActivity.addContentView() was such a method.

And it had appeared as a regression from our MSBuild tests[*1].

Fixes:

- removed dependency on BaseGen in ApiXmlAdjuster. Use BaseType instead,
  which doesn't depend on validation step.
  - Though this change uncovered an issue that Ctor and Parameter needed
    to hold Java type information. So, made some changes to them.
- Now that ApiXmlAdjuster is getting valid managed type hierarchy, it
  had uncovered another kind of issues: managed objects need to exist
  to not cause some null crashes. Hence there is ManagedType class now.
  - Type resolution now premises this ManagedType in type resolver code.
- Fixed incorrect Load() invocation for ClassGen and InterfaceGen (that
  had caused missing BaseType information).
- Added (almost irrelevant) ApiXmlAdjuster test to make sure override
  resolution is done for overrides from indirect inheritance.

[*1] that needs to be OSS-ed...
@atsushieno
Copy link
Contributor Author

This also seems to fix https://bugzilla.xamarin.com/show_bug.cgi?id=44151

@atsushieno
Copy link
Contributor Author

@dellis1972
Copy link
Contributor

@atsushieno this looks ok to me. Will wait for @jonpryor to comment before merging though

@jonpryor
Copy link
Contributor

Consensus is to hold off on merging this PR until our internal API comparison tools work again, to help ensure that this fix doesn't break anything.

Unfortunately our internal API comparison tools are "broken" because of changes to mono-api-info and mono-api-html, which are still in-progress...

@jonpryor jonpryor merged commit de64e7b into dotnet:master Oct 3, 2016
jonpryor added a commit to jonpryor/java.interop that referenced this pull request Jun 4, 2020
Changes: dotnet/android-tools@f5fcb9f...3974fc3

  * dotnet/android-tools@3974fc3: [Xamarin.Android.Tools.AndroidSdk] JdkInfo + JDK11 + Windows (dotnet#88)
  * dotnet/android-tools@5552b07: [Xamarin.Android.Tools.AndroidSdk] Improve utility of JDK warnings (dotnet#87)
  * dotnet/android-tools@13cc497: [Xamarin.Android.Tools.AndroidSdk] Prefer JAVA_HOME (dotnet#86)
  * dotnet/android-tools@967c278: Delete NuGet.Config
  * dotnet/android-tools@2d3690e: [Xamarin.Android.Tools.AndroidSdk] Nullable Reference Type support (dotnet#84)
  * dotnet/android-tools@2020b20: [Xamarin.Android.Tools.AndroidSdk] Preview Build-Tools are Last (dotnet#85)
jonpryor added a commit that referenced this pull request Jun 4, 2020
Changes: dotnet/android-tools@f5fcb9f...3974fc3

  * dotnet/android-tools@3974fc3: [Xamarin.Android.Tools.AndroidSdk] JdkInfo + JDK11 + Windows (#88)
  * dotnet/android-tools@5552b07: [Xamarin.Android.Tools.AndroidSdk] Improve utility of JDK warnings (#87)
  * dotnet/android-tools@13cc497: [Xamarin.Android.Tools.AndroidSdk] Prefer JAVA_HOME (#86)
  * dotnet/android-tools@967c278: Delete NuGet.Config
  * dotnet/android-tools@2d3690e: [Xamarin.Android.Tools.AndroidSdk] Nullable Reference Type support (#84)
  * dotnet/android-tools@2020b20: [Xamarin.Android.Tools.AndroidSdk] Preview Build-Tools are Last (#85)
jonpryor added a commit that referenced this pull request Jun 11, 2020
Changes: dotnet/android-tools@23c4fe0...017078f

  * dotnet/android-tools@017078f: [Xamarin.Android.Tools.AndroidSdk] JdkInfo + JDK11 + Windows (#88)
  * dotnet/android-tools@852e4a3: [Xamarin.Android.Tools.AndroidSdk] Improve utility of JDK warnings (#87)
  * dotnet/android-tools@b055edf: [Xamarin.Android.Tools.AndroidSdk] Prefer JI_JAVA_HOME (#86)
  * dotnet/android-tools@ef31658: [Xamarin.Android.Tools.AndroidSdk] Nullable Reference Type support (#84)
  * dotnet/android-tools@b8efdae: [Xamarin.Android.Tools.AndroidSdk] Preview Build-Tools are Last (#85)
@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 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.

4 participants