Skip to content
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

[generator] Better support aliased types #173

Merged
merged 1 commit into from Aug 8, 2017

Conversation

@jonpryor
Copy link
Member

commented Aug 4, 2017

Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=56436

generator relies on the existance of a mapping from Java types to
managed types which bind the Java type. The generator --ref option
will cause generator to make this mapping based on the managed types
from an assembly, and the mapping in turn is handled by [Register]
and related custom attributes:

    namespace Android.Runtime {
      [Register ("java/util/Collection", DoNotRegisterAcw=true)]
      partial class JavaCollection {
      }
    }

For better or worse, nothing enforces that there be only one such
mapping, It is thus possible to have multiple types partake in the
mapping process:

    namespace Java.Util {
      [Register ("java/util/Collection", DoNotRegisterAcw=true)]
      partial interface ICollection {
      }
    }

A Java type is aliased when two or more managed types bind the same
Java type, as with the above Android.Runtime.JavaCollection and
Java.Util.ICollection types;

This is a scenario which as long existed.

Question: What does generator do when a type is aliased?
Answer: generator registers mappings from assemblies in the order of
types in the assembly it's processing. When an aliased type is found,
the last registered type "wins".

For example, if an assembly defines JavaCollection before
ICollection, then a type lookup for java.util.Collection will
return ICollection. If instead the assembly defines ICollection
before JavaCollection, then a type lookup for java.util.Collection
willl instead return JavaCollection.

This is not necessarily desirable, but I don't see much alternative.

Unfortunately, that's only half the scenario. There are a number of
situations in which SymbolTable.Lookup() is not given a Java name,
but is instead given the managed binding type name. In particular,
this happens when ManagedClassGen.Validate() is called, and it
attempts to validate the base class and all implemented interfaces,
using the managed type names for this process.

Here is where the "last registered type wins" behavior becomes
problematic, as it means that a Lookup for Java.Util.ICollection can
fail when JavaCollection is registered after ICollection, because
the Java name was shared.

Which brings us to Bug #56436: building a Xamarin.Android binding
project changes behavior based on whether Xamarin.Android 7.2 or
Xamarin.Android 7.3 is used, and the primary difference between these
differences is the order that types are defined in Mono.Android.dll.
In XA 7.2, JavaCollection is defined before ICollection.
In XA 7.3, ICollection is defined before JavaCollection.
This (subtle!) change interacts with generator, and within XA 7.3
means that Java.Util.ICollection can't be found:

    warning BG8C00: For type Java.Util.AbstractQueue, base interface Java.Util.ICollection does not exist.

Improve this state of affairs by changing SymbolTable.symbols from a
Dictionary<string, ISymbol> to a Dictionary<string, List<ISymbol>>,
allowing generator to preserve all the managed types which bind a
given Java type. This in turn allows both ICollection and
JavaCollection to be preserved, so that a lookup for
Java.Util.ICollection will now work consistently, without any type
definition ordering issues.

Unfortunately lookups for the Java type java.util.Collection will
still be order-dependent, continuing to return the last registered
type. Hopefully this won't cause any problems.

@jonpryor jonpryor force-pushed the jonp-generator-type-aliases branch from c37d645 to f076c7c Aug 4, 2017

@jonpryor jonpryor requested a review from atsushieno Aug 4, 2017

[generator] Better support aliased types
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=56436

`generator` relies on the existance of a mapping from Java types to
managed types which bind the Java type. The `generator --ref` option
will cause `generator` to make this mapping based on the managed types
from an assembly, and the mapping in turn is handled by `[Register]`
and related custom attributes:

	namespace Android.Runtime {
	  [Register ("java/util/Collection", DoNotRegisterAcw=true)]
	  partial class JavaCollection {
	  }
	}

For better or worse, nothing enforces that there be only one such
mapping, It is thus possible to have multiple types partake in the
mapping process:

	namespace Java.Util {
	  [Register ("java/util/Collection", DoNotRegisterAcw=true)]
	  partial interface ICollection {
	  }
	}

A Java type is *aliased* when two or more managed types bind the same
Java type, as with the above `Android.Runtime.JavaCollection` and
`Java.Util.ICollection` types;

This is a scenario which as long existed.

Question: What does `generator` do when a type is aliased?
Answer: `generator` registers mappings from assemblies in the order of
types in the assembly it's processing. When an aliased type is found,
the *last registered type* "wins".

For example, if an assembly defines `JavaCollection` before
`ICollection`, then a type lookup for `java.util.Collection` will
return `ICollection`. If instead the assembly defines `ICollection`
before `JavaCollection`, then a type lookup for `java.util.Collection`
willl instead return `JavaCollection`.

This is not necessarily desirable, but I don't see much alternative.

Unfortunately, that's only *half* the scenario. There are a number of
situations in which `SymbolTable.Lookup()` is not given a Java name,
but is instead given the *managed binding type name*. In particular,
this happens when `ManagedClassGen.Validate()` is called, and it
attempts to validate the base class and all implemented interfaces,
using the *managed* type names for this process.

Here is where the "last registered type wins" behavior becomes
problematic, as it means that a Lookup for `Java.Util.ICollection` can
fail when `JavaCollection` is registered after `ICollection`, because
the Java name was shared.

Which brings us to Bug #56436: building a Xamarin.Android binding
project changes behavior based on whether Xamarin.Android 7.2 or
Xamarin.Android 7.3 is used, and the primary difference between these
differences is the order that types are defined in `Mono.Android.dll`.
In XA 7.2, `JavaCollection` is defined before `ICollection`.
In XA 7.3, `ICollection` is defined before `JavaCollection`.
This (subtle!) change interacts with `generator`, and within XA 7.3
means that `Java.Util.ICollection` can't be found:

	warning BG8C00: For type Java.Util.AbstractQueue, base interface Java.Util.ICollection does not exist.

Improve this state of affairs by changing `SymbolTable.symbols` from a
`Dictionary<string, ISymbol>` to a `Dictionary<string, List<ISymbol>>`,
allowing `generator` to preserve all the managed types which bind a
given Java type. This in turn allows *both* `ICollection` and
`JavaCollection` to be preserved, so that a lookup for
`Java.Util.ICollection` will now work consistently, without any type
definition ordering issues.

Unfortunately lookups for the Java type `java.util.Collection` will
still be order-dependent, continuing to return the *last* registered
type.  Hopefully this won't cause any problems.

@jonpryor jonpryor force-pushed the jonp-generator-type-aliases branch from f076c7c to 2194c90 Aug 4, 2017

@atsushieno atsushieno merged commit d7dfa0b into master Aug 8, 2017

1 check passed

Build Build finished. 751 tests run, 2 skipped, 0 failed.
Details
jonpryor added a commit that referenced this pull request Aug 10, 2017
[generator] Better support aliased types (#173)
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=56436

`generator` relies on the existance of a mapping from Java types to
managed types which bind the Java type. The `generator --ref` option
will cause `generator` to make this mapping based on the managed types
from an assembly, and the mapping in turn is handled by `[Register]`
and related custom attributes:

	namespace Android.Runtime {
	  [Register ("java/util/Collection", DoNotRegisterAcw=true)]
	  partial class JavaCollection {
	  }
	}

For better or worse, nothing enforces that there be only one such
mapping, It is thus possible to have multiple types partake in the
mapping process:

	namespace Java.Util {
	  [Register ("java/util/Collection", DoNotRegisterAcw=true)]
	  partial interface ICollection {
	  }
	}

A Java type is *aliased* when two or more managed types bind the same
Java type, as with the above `Android.Runtime.JavaCollection` and
`Java.Util.ICollection` types;

This is a scenario which as long existed.

Question: What does `generator` do when a type is aliased?
Answer: `generator` registers mappings from assemblies in the order of
types in the assembly it's processing. When an aliased type is found,
the *last registered type* "wins".

For example, if an assembly defines `JavaCollection` before
`ICollection`, then a type lookup for `java.util.Collection` will
return `ICollection`. If instead the assembly defines `ICollection`
before `JavaCollection`, then a type lookup for `java.util.Collection`
willl instead return `JavaCollection`.

This is not necessarily desirable, but I don't see much alternative.

Unfortunately, that's only *half* the scenario. There are a number of
situations in which `SymbolTable.Lookup()` is not given a Java name,
but is instead given the *managed binding type name*. In particular,
this happens when `ManagedClassGen.Validate()` is called, and it
attempts to validate the base class and all implemented interfaces,
using the *managed* type names for this process.

Here is where the "last registered type wins" behavior becomes
problematic, as it means that a Lookup for `Java.Util.ICollection` can
fail when `JavaCollection` is registered after `ICollection`, because
the Java name was shared.

Which brings us to Bug #56436: building a Xamarin.Android binding
project changes behavior based on whether Xamarin.Android 7.2 or
Xamarin.Android 7.3 is used, and the primary difference between these
differences is the order that types are defined in `Mono.Android.dll`.
In XA 7.2, `JavaCollection` is defined before `ICollection`.
In XA 7.3, `ICollection` is defined before `JavaCollection`.
This (subtle!) change interacts with `generator`, and within XA 7.3
means that `Java.Util.ICollection` can't be found:

	warning BG8C00: For type Java.Util.AbstractQueue, base interface Java.Util.ICollection does not exist.

Improve this state of affairs by changing `SymbolTable.symbols` from a
`Dictionary<string, ISymbol>` to a `Dictionary<string, List<ISymbol>>`,
allowing `generator` to preserve all the managed types which bind a
given Java type. This in turn allows *both* `ICollection` and
`JavaCollection` to be preserved, so that a lookup for
`Java.Util.ICollection` will now work consistently, without any type
definition ordering issues.

Unfortunately lookups for the Java type `java.util.Collection` will
still be order-dependent, continuing to return the *last* registered
type.  Hopefully this won't cause any problems.

@jpobst jpobst deleted the jonp-generator-type-aliases branch Aug 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.