Skip to content

Commit

Permalink
[generator] Don't special case synthetic methods
Browse files Browse the repository at this point in the history
Context: xamarin/xamarin-android#1998 (comment)
Context: https://github.com/xamarin/monodroid/commit/2c515356d9a671f729aff2e81c4c2f6184749722

Long ago, `generator` was written to consume API XML descriptions
provided by Google.  This approach worked reasonably well until
Honeycomb, at which point Google didn't provide an XML API description
when the API-H `android.jar` was released.

The fix was `jar2xml`, which used Java Reflection around `android.jar`
to generate a similar API XML description which `generator` could use.

`jar2xml` in turn had its own share of problems, which resulted in
`class-parse`, which had a *different* share of problems. :-)

The original hope was that `generator` could directly use
`class-parse` output.  Turns out, it wasn't that easy, hence
`Xamarin.Android.Tools.ApiXmlAdjuster`, which took `class-parse`
output and "munged" it into something resembling `jar2xml` output
(e.g. with respect to whether method overrides were included).

However, *before* `Xamarin.Android.Tools.ApiXmlAdjuster` existed, I
tried to get `generator` to consume `class-parse` output directly, and
in that spirit monodroid@2c515356 updated `generator` to *skip*
processing of `synthetic` methods:

> Don't emit `synthetic` methods, as this results in "duplicate" methods
> -- one for the "real" method, one for the synthetic method.

However, the scenario monodroid@2c515356 attempted to address --
direct support for `class-parse` output -- is not something that has
ever been used, or been tested, or even *worked*.

Thus, the special-casing of `synthetic` methods is effectively dead
code, all the more so because `Xamarin.Android.Tools.ApiXmlAdjuster`
never emitted the `//method/@synthetic` attribute in the first place,
so `generator`s special-casing of `synthetic` methods was never hit.

...until 0881acc, which updated
`Xamarin.Android.Tools.ApiXmlAdjuster` to include and copy over *all*
source attributes, *including* `//method/@synthetic`.

*NOW* `generator` began to see methods with
`//method[@Synthetic='true']`, *and **skip** them*.

The resulting code *compiled*, but had [ABI breakage][0].

Remove the special-casing logic for `synthetic` methods from
`generator`.  monodroid@2c515356 never supported direct consumption
of `class-parse` output by `generator`, meaning such an effort will
require more work *anyway*, and removing `synthetic` special-casing from
`generator` will allow it to consume post-0881accd
`Xamarin.Android.Tools.ApiXmlAdjuster` output without breaking things.

[0]: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/3642/API_20Compatibility_20Checks/
  • Loading branch information
jonpryor authored and atsushieno committed Jul 27, 2018
1 parent 9fecba2 commit 78f7301
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 7 deletions.
6 changes: 1 addition & 5 deletions tools/generator/ClassGen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,7 @@ public XmlClassGen (XElement pkg, XElement elem)
AddInterface (iname);
break;
case "method":
var synthetic = child.XGetAttribute ("synthetic") == "true";
var finalizer = child.XGetAttribute ("name") == "finalize" &&
child.XGetAttribute ("jni-signature") == "()V";
if (!(synthetic || finalizer))
AddMethod (new XmlMethod (this, child));
AddMethod (new XmlMethod (this, child));
break;
case "constructor":
Ctors.Add (new XmlCtor (this, child));
Expand Down
3 changes: 1 addition & 2 deletions tools/generator/InterfaceGen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ public XmlInterfaceGen (XElement pkg, XElement elem)
AddInterface (iname);
break;
case "method":
if (child.XGetAttribute ("synthetic") != "true")
AddMethod (new XmlMethod (this, child));
AddMethod (new XmlMethod (this, child));
break;
case "field":
AddField (new XmlField (child));
Expand Down

0 comments on commit 78f7301

Please sign in to comment.