-
Notifications
You must be signed in to change notification settings - Fork 63
[generator][api-xml-adjuster] Fix managed type resolution in API adjuster. #86
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
jonpryor
merged 1 commit into
dotnet:master
from
atsushieno:fix-api-adjuster-type-resolution
Oct 3, 2016
Merged
[generator][api-xml-adjuster] Fix managed type resolution in API adjuster. #86
jonpryor
merged 1 commit into
dotnet:master
from
atsushieno:fix-api-adjuster-type-resolution
Oct 3, 2016
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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...
Contributor
Author
Contributor
Author
Contributor
|
@atsushieno this looks ok to me. Will wait for @jonpryor to comment before merging though |
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
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)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
which doesn't depend on validation step.
to hold Java type information. So, made some changes to them.
had uncovered another kind of issues: managed objects need to exist
to not cause some null crashes. Hence there is ManagedType class now.
had caused missing BaseType information).
resolution is done for overrides from indirect inheritance.
[*1] that needs to be OSS-ed...