Skip to content

Commit 3343634

Browse files
jonpryoratsushieno
authored andcommitted
[generator] Only skip generic "overloads" of bound types (#178)
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=57828 The #runtime team is attempting to update xamarin-android to use the [mono/2017-06][0] and [mono/2017-08][1] (and...) branches, and when doing so they are seeing a unit test fail in `Xamarin.Android.Build.Tests.BuildTest.GeneratorValidateMultiMethodEventName()`: no `BG850*` warnings are expected but they are produced. After much analysis, the cause of ght `BG850*` warnings is that the mono/2017-06 branch introduces a non-generic "overload" of `System.Collections.Generic.KeyValuePair`: namespace System.Collections.Generic { // New with mono/2017-06 partial class KeyValuePair { public static KeyValuePair<TKey, TValue> Create<TKey, TValue>(TKey key, TValue value); } // Existing since forever partial struct KeyValuePair<TKey, TValue> { } } Specifically, the above "`KeyValuePair` overload" type runs into a FIXME within `CodeGenerator.cs`: // FIXME: at some stage we want to import generic types. // For now generator fails to load generic types that have conflicting type e.g. // AdapterView`1 and AdapterView cannot co-exist. // It is mostly because generator primarily targets jar (no real generics land). The *intent* of the check that is that, given a type `AdapterView<T>`, we only check to see if `AdapterView` exists as well. If it does exist, then we *ignore* the `AdapterView<T>` type, and only use the `AdapterView` type for code generation. In the case of `KeyValuePair`, the same "check" is done, which cause us to *skip* type registration for `KeyValuePair<TKey, TValue>`, which in turn causes us to invalidate e.g. `IDictionary<TKey, TValue>` -- as it implements `ICollection<KeyValuePair<TKey, TValue>>` -- and everything promply falls apart from there: warning BG8C00: For type System.Collections.Generic.IDictionary`2, base interface System.Collections.Generic.ICollection`1<System.Collections.Generic.KeyValuePair`2<TKey,TValue>> is invalid. warning BG8C00: For type System.Collections.Generic.IDictionary`2, base interface System.Collections.Generic.IEnumerable`1<System.Collections.Generic.KeyValuePair`2<TKey,TValue>> is invalid. warning BG8502: Invalidating System.Collections.Generic.IDictionary`2 and all nested types because some of its interfaces were invalid. warning BG8801: Invalid parameter type System.Collections.Generic.IDictionary`2<System.String,System.Collections.Generic.IList`1<System.String>> in method MapEquivalents in managed type Java.Util.Locale.LanguageRange. warning BG8801: Invalid parameter type System.Collections.Generic.IDictionary`2<System.String,System.Collections.Generic.IList`1<System.String>> in method Parse in managed type Java.Util.Locale.LanguageRange. warning BG8801: Invalid parameter type System.Collections.Generic.IDictionary`2<System.String,System.Object> in method NewFileSystem in managed type Java.Nio.FileNio.Spi.FileSystemProvider. warning BG8801: Invalid parameter type System.Collections.Generic.IDictionary`2<System.String,System.Object> in method NewFileSystem in managed type Java.Nio.FileNio.Spi.FileSystemProvider. warning BG8801: Invalid parameter type System.Collections.Generic.IDictionary`2<System.String,System.String> in method .ctor in managed type Java.Security.Provider.Service. warning BG8801: Invalid parameter type System.Collections.Generic.IDictionary`2<System.Object,System.Object> in method PutAll in managed type Java.Security.Provider. I still don't fully understand the scenario that the check was attempting to solve, so in lieu of actually fixing the FIXME, make the check *stricter* so that we only ignore "generically overloaded" types if they bind the *same* Java type. Since `KeyValuePair<TKey, TValue>` binds *no* Java type, it will no longer be skipped, thus removing the BG8502 and related warnings. Additionally, a bit of code cleanup for consistency: some parts of the code use `HAVE_CECIL`, and others use `USE_CECIL`. Standardize on `HAVE_CECIL` for consistency and sanity. [0]: dotnet/android#631 [1]: dotnet/android#723
1 parent 1cd0361 commit 3343634

File tree

11 files changed

+38
-25
lines changed

11 files changed

+38
-25
lines changed

src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JniType.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
#if HAVE_CECIL
1010
using Mono.Cecil;
1111
using Java.Interop.Tools.Cecil;
12-
#if !GENERATOR
1312
using Android.Runtime;
13+
#if !GENERATOR
1414
using Java.Interop.Tools.JavaCallableWrappers;
1515
#endif // !GENERATOR
1616
#endif // HAVE_CECIL
@@ -252,7 +252,7 @@ static string GetSpecialExportJniType (string typeName, ExportParameterKind expo
252252
return null;
253253
}
254254

255-
#if !GEN_JAVA_STUBS && !GENERATOR && !JAVADOC_TO_MDOC
255+
#if !GEN_JAVA_STUBS && !JAVADOC_TO_MDOC
256256
// Keep in sync with ToJniNameFromAttributes(TypeDefinition)
257257
public static string ToJniNameFromAttributes (Type type)
258258
{
@@ -384,7 +384,7 @@ static string ToJniNameWhichShouldReplaceExistingToJniName (Type type, ExportPar
384384
}
385385
#endif
386386

387-
#if HAVE_CECIL && !GENERATOR
387+
#if HAVE_CECIL
388388

389389
internal static ExportParameterKind GetExportKind (Mono.Cecil.ICustomAttributeProvider p)
390390
{

tools/generator/ClassGen.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
using System.Xml.Linq;
1616

1717
namespace MonoDroid.Generation {
18-
#if USE_CECIL
18+
#if HAVE_CECIL
1919
static class ManagedExtensions
2020
{
2121
public static string FullNameCorrected (this TypeReference t)
@@ -69,7 +69,7 @@ public override bool IsFinal {
6969
get { return t.IsSealed; }
7070
}
7171
}
72-
#endif
72+
#endif // HAVE_CECIL
7373

7474
public class XmlClassGen : ClassGen {
7575
bool is_abstract;

tools/generator/CodeGenerator.cs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Xamarin.Android.Tools.ApiXmlAdjuster;
1515

1616
using Java.Interop.Tools.Cecil;
17+
using Java.Interop.Tools.TypeNameMappings;
1718

1819
namespace Xamarin.Android.Binder {
1920

@@ -262,8 +263,10 @@ static void Run (CodeGeneratorOptions options, DirectoryAssemblyResolver resolve
262263
// For now generator fails to load generic types that have conflicting type e.g.
263264
// AdapterView`1 and AdapterView cannot co-exist.
264265
// It is mostly because generator primarily targets jar (no real generics land).
265-
if (td.HasGenericParameters &&
266-
md.GetType (td.FullName.Substring (0, td.FullName.IndexOf ('`'))) != null)
266+
var nonGenericOverload = td.HasGenericParameters
267+
? md.GetType (td.FullName.Substring (0, td.FullName.IndexOf ('`')))
268+
: null;
269+
if (BindSameType (td, nonGenericOverload))
267270
continue;
268271
ProcessReferencedType (td, opt);
269272
}
@@ -366,6 +369,15 @@ static void AddTypeToTable (GenBase gb)
366369
AddTypeToTable (nt);
367370
}
368371

372+
static bool BindSameType (TypeDefinition a, TypeDefinition b)
373+
{
374+
if (a == null || b == null)
375+
return false;
376+
if (!a.ImplementsInterface ("Android.Runtime.IJavaObject") || !b.ImplementsInterface ("Android.Runtime.IJavaObject"))
377+
return false;
378+
return JniType.ToJniName (a) == JniType.ToJniName (b);
379+
}
380+
369381
static IEnumerable<GenBase> FlattenNestedTypes (IEnumerable<GenBase> gens)
370382
{
371383
foreach (var g in gens) {
@@ -404,7 +416,7 @@ static void Validate (List<GenBase> gens, CodeGenerationOptions opt)
404416
} while (removed.Count > 0);
405417
}
406418

407-
#if USE_CECIL
419+
#if HAVE_CECIL
408420
static void ProcessReferencedType (TypeDefinition td, CodeGenerationOptions opt)
409421
{
410422
if (!td.IsPublic && !td.IsNested)
@@ -434,7 +446,7 @@ static void ProcessReferencedType (TypeDefinition td, CodeGenerationOptions opt)
434446
foreach (var nt in td.NestedTypes)
435447
ProcessReferencedType (nt, opt);
436448
}
437-
#endif
449+
#endif // HAVE_CECIL
438450

439451
static void GenerateAnnotationAttributes (List<GenBase> gens, IEnumerable<string> zips)
440452
{

tools/generator/Ctor.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
using Xamarin.Android.Tools;
99

1010
namespace MonoDroid.Generation {
11-
#if USE_CECIL
11+
#if HAVE_CECIL
1212
public class ManagedCtor : Ctor {
1313
MethodDefinition m;
1414
string name;
@@ -53,7 +53,7 @@ public override string CustomAttributes {
5353
get { return null; }
5454
}
5555
}
56-
#endif
56+
#endif // HAVE_CECIL
5757

5858
public class XmlCtor : Ctor {
5959
string name;

tools/generator/Field.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
using System.Xml.Linq;
1212

1313
namespace MonoDroid.Generation {
14-
#if USE_CECIL
14+
#if HAVE_CECIL
1515
public class ManagedField : Field {
1616
FieldDefinition f;
1717
string java_name;
@@ -83,7 +83,7 @@ protected override Parameter SetterParameter {
8383
}
8484
}
8585
}
86-
#endif
86+
#endif // HAVE_CECIL
8787

8888
public class XmlField : Field {
8989

tools/generator/GenBaseSupport.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public static bool IsPrefixableName (string name)
5050
}
5151
}
5252

53-
#if USE_CECIL
53+
#if HAVE_CECIL
5454
public class ManagedGenBaseSupport : GenBaseSupport
5555
{
5656
TypeDefinition t;
@@ -152,7 +152,7 @@ public override string Visibility {
152152
get { return t.IsPublic || t.IsNestedPublic ? "public" : "protected internal"; }
153153
}
154154
}
155-
#endif
155+
#endif // HAVE_CECIL
156156

157157
public class XmlGenBaseSupport : GenBaseSupport
158158
{

tools/generator/InterfaceGen.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
using Xamarin.Android.Tools;
1212

1313
namespace MonoDroid.Generation {
14-
#if USE_CECIL
14+
#if HAVE_CECIL
1515
public class ManagedInterfaceGen : InterfaceGen {
1616
public ManagedInterfaceGen (TypeDefinition t)
1717
: base (new ManagedGenBaseSupport (t))
@@ -34,7 +34,7 @@ public override bool MayHaveManagedGenericArguments {
3434
get { return !this.IsAcw; }
3535
}
3636
}
37-
#endif
37+
#endif // HAVE_CECIL
3838

3939
public class XmlInterfaceGen : InterfaceGen {
4040

tools/generator/Method.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
using System.Xml.Linq;
1313

1414
namespace MonoDroid.Generation {
15-
#if USE_CECIL
15+
#if HAVE_CECIL
1616
public class ManagedMethod : Method {
1717
MethodDefinition m;
1818
string java_name;
@@ -121,7 +121,7 @@ public override string CustomAttributes {
121121
get { return null; }
122122
}
123123
}
124-
#endif
124+
#endif // HAVE_CECIL
125125

126126
public class XmlMethod : Method {
127127

tools/generator/MethodBase.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public interface IMethodBaseSupport {
2020
string Visibility { get; }
2121
}
2222

23-
#if USE_CECIL
23+
#if HAVE_CECIL
2424
public class ManagedMethodBaseSupport : IMethodBaseSupport {
2525
MethodDefinition m;
2626
public ManagedMethodBaseSupport (MethodDefinition m)
@@ -79,7 +79,7 @@ public IEnumerable<Parameter> GetParameters (CustomAttribute regatt)
7979
}
8080
}
8181
}
82-
#endif
82+
#endif // HAVE_CECIL
8383

8484
public class XmlMethodBaseSupport : IMethodBaseSupport {
8585

tools/generator/Parameter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ public static Parameter FromClassElement (XElement elem)
284284
return new Parameter (name, java_package + "." + java_type, null, false);
285285
}
286286

287-
#if USE_CECIL
287+
#if HAVE_CECIL
288288
public static Parameter FromManagedParameter (ParameterDefinition p, string jnitype, string rawtype)
289289
{
290290
// FIXME: safe to use CLR type name? assuming yes as we often use it in metadatamap.
@@ -297,6 +297,6 @@ public static Parameter FromManagedType (TypeDefinition t, string javaType)
297297
{
298298
return new Parameter ("__self", javaType ?? t.FullName, t.FullName, false);
299299
}
300-
#endif
300+
#endif // HAVE_CECIL
301301
}
302302
}

0 commit comments

Comments
 (0)