Fixing various AOT bugs (#1648)

* Fix missing winrt check before generating vtable entry for key value pair

* Add more tests

* Fix type in MapChangedEventArgs

* Fix issue with SingleItemReadOnlyList

* Fix handling of nested and generic types

* Fix merge

src/Authoring/WinRT.SourceGenerator/AotOptimizer.cs

@@ -240,17 +240,51 @@ private static string ToFullyQualifiedString(ISymbol symbol)
 
         private static string ToVtableLookupString(ISymbol symbol)
         {
-            if (symbol is INamedTypeSymbol namedTypeSymbol && 
+            List<string> genericArguments = [];
+            var fullName = ToVtableLookupString(symbol, genericArguments);
+            if (genericArguments.Count == 0)
+            {
+                return fullName;
+            }
+
+            return $$"""{{fullName}}[{{string.Join(",", genericArguments)}}]""";
+        }
+
+        private static string ToVtableLookupString(ISymbol symbol, List<string> genericArguments, bool ignoreTypeArguments = false)
+        {
+            if (symbol is INamedTypeSymbol namedTypeSymbol &&
+                !ignoreTypeArguments &&
                 namedTypeSymbol.TypeArguments.Length != 0)
             {
-                return $$"""{{symbol.ContainingNamespace?.ToDisplayString()}}.{{symbol.MetadataName}}[{{string.Join(",", namedTypeSymbol.TypeArguments.Select(ToVtableLookupString))}}]""";
+                // Ignore type arguments and get the string representation for the rest of
+                // the type to properly handle nested types.
+                var fullName = ToVtableLookupString(symbol, genericArguments, true);
+                // Type arguments are collected but not added to the type name until the end
+                // per the format of Type.ToString().  ToVtableLookupString on the symbol is
+                // also called first to ensure any type arguments from any nested parent types
+                // are added first.
+                foreach (var typeArgument in namedTypeSymbol.TypeArguments)
+                {
+                    genericArguments.Add(ToVtableLookupString(typeArgument));
+                }
+                return fullName;
             }
             else
             {
-                var symbolDisplayString = new SymbolDisplayFormat(
-                    globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Omitted,
-                    typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces);
-                return symbol.ToDisplayString(symbolDisplayString);
+                // If it is a generic type argument or the type is directly under a namspace, we just use the ToDisplayString API.
+                if (symbol is not INamedTypeSymbol || symbol.ContainingSymbol is INamespaceSymbol || symbol.ContainingSymbol is null)
+                {
+                    var arity = symbol is INamedTypeSymbol namedType && namedType.Arity > 0 ? "`" + namedType.Arity : "";
+                    var symbolDisplayString = new SymbolDisplayFormat(
+                        globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Omitted,
+                        typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces);
+                    return symbol.ToDisplayString(symbolDisplayString) + arity;
+                }
+                else
+                {
+                    // Nested types use + in the fully qualified name rather than .
+                    return ToVtableLookupString(symbol.ContainingSymbol, genericArguments) + "+" + symbol.MetadataName;
+                }
             }
         }
 
@@ -382,7 +416,7 @@ internal static VtableAttribute GetVtableAttributeToAdd(
             }
 
             // KeyValueType is a value type in C#, but it is projected as a reference type in WinRT. 
-            if (symbol.TypeKind == TypeKind.Struct && symbol.MetadataName == "KeyValuePair`2")
+            if (symbol.TypeKind == TypeKind.Struct && symbol.MetadataName == "KeyValuePair`2" && isWinRTType(symbol, mapper))
             {
                 interfacesToAddToVtable.Add(ToFullyQualifiedString(symbol));
                 AddGenericInterfaceInstantiation(symbol as INamedTypeSymbol);
@@ -1100,6 +1134,27 @@ internal static void AddVtableAdapterTypeForKnownInterface(ITypeSymbol classType
                 }
             }
 
+            if (classType is INamedTypeSymbol namedType && namedType.MetadataName == "ObservableCollection`1")
+            {
+                // ObservableCollection make use of an internal built-in type as part of its
+                // implementation for INotifyPropertyChanged.  Handling that manually here.
+                var genericInterfaces = new List<string>() { "System.Collections.IList", "System.Collections.IEnumerable" };
+                var mapping = mapper.GetMappedType("System.Collections.IList").GetMapping();
+                vtableAttributes.Add(
+                    new VtableAttribute(
+                        "System.Collections.Specialized",
+                        false,
+                        "SingleItemReadOnlyList",
+                        ImmutableArray<TypeInfo>.Empty,
+                        "System.Collections.Specialized.SingleItemReadOnlyList",
+                        genericInterfaces.ToImmutableArray(),
+                        ImmutableArray<GenericInterface>.Empty,
+                        false,
+                        false,
+                        false,
+                        mapping.Item1 + "." + mapping.Item2));
+            }
+
             void LookupAndAddVtableAttributeForGenericType(string type, ImmutableArray<ITypeSymbol> genericArgs)
             {
                 var genericType = compilation.GetTypeByMetadataName(type);

src/Authoring/WinRT.SourceGenerator/GenericVtableInitializerStrings.cs

@@ -111,9 +111,9 @@ public static string GetInstantiation(string genericInterface, EquatableArray<Ge
             {
                 return GetIAsyncOperationInstantiation(genericParameters);
             }
-            else if (genericInterface == "Windows.Foundation.Collections.IMapedChangedEventArgs`1")
+            else if (genericInterface == "Windows.Foundation.Collections.IMapChangedEventArgs`1")
             {
-                return GetIMapedChangedEventArgsInstantiation(genericParameters);
+                return GetIMapChangedEventArgsInstantiation(genericParameters);
             }
             else if (genericInterface == "Windows.Foundation.Collections.IObservableMap`2")
             {
@@ -1546,12 +1546,12 @@ private static unsafe int Do_Abi_get_Completed_1(IntPtr thisPtr, IntPtr* __retur
             return instantiation;
         }
 
-        private static string GetIMapedChangedEventArgsInstantiation(EquatableArray<GenericParameter> genericParameters)
+        private static string GetIMapChangedEventArgsInstantiation(EquatableArray<GenericParameter> genericParameters)
         {
-            string staticMethodsClass = $"global::ABI.Windows.Foundation.Collections.IMapedChangedEventArgsMethods<{GetGenericParametersAsString(genericParameters, ", ", false, false)}>";
-            string abiStaticMethodsClass = $"global::ABI.Windows.Foundation.Collections.IMapedChangedEventArgsMethods<{GetGenericParametersAsString(genericParameters, ", ", true, false)}>";
+            string staticMethodsClass = $"global::ABI.Windows.Foundation.Collections.IMapChangedEventArgsMethods<{GetGenericParametersAsString(genericParameters, ", ", false, false)}>";
+            string abiStaticMethodsClass = $"global::ABI.Windows.Foundation.Collections.IMapChangedEventArgsMethods<{GetGenericParametersAsString(genericParameters, ", ", true, false)}>";
             string instantiation = $$"""
-             internal static class IMapedChangedEventArgs_{{GetGenericParametersAsString(genericParameters, "_", false)}}
+             internal static class IMapChangedEventArgs_{{GetGenericParametersAsString(genericParameters, "_", false)}}
              {
                  private static bool _initialized = Init();
                  internal static bool Initialized => _initialized;

src/Tests/FunctionalTests/CCW/Program.cs

@@ -187,11 +187,12 @@
     return 124;
 }
 
+TestClass.TestNestedClass();
+
 // These scenarios aren't supported today on AOT, but testing to ensure they
 // compile without issues.  They should still work fine outside of AOT.
 try
 {
-    TestClass.TestNestedClass();
     TestClass.TestGenericList<bool>();
 }
 catch(Exception)
@@ -416,11 +417,27 @@ IEnumerator IEnumerable.GetEnumerator()
 
 sealed class TestClass
 {
+    // Testing various nested and generic classes on vtable lookup table.
     public static void TestNestedClass()
     {
         var instance = new Class();
         var nestedClassList = new List<NestedClass>();
         instance.BindableIterableProperty = nestedClassList;
+
+        var nestedClassList2 = new List<NestedClass.NestedClass2>();
+        instance.BindableIterableProperty = nestedClassList2;
+
+        var nestedClassList3 = new List<NestedGenericClass<int>>();
+        instance.BindableIterableProperty = nestedClassList3;
+
+        var nestedClassList4 = new List<NestedGenericClass<int>.NestedClass2>();
+        instance.BindableIterableProperty = nestedClassList4;
+
+        var nestedClassList5 = new List<NestedGenericClass<NestedGenericClass<int>.NestedClass2>.NestedClass2>();
+        instance.BindableIterableProperty = nestedClassList5;
+
+        var nestedClassList6 = new List<NestedGenericClass<int>.NestedClass3<double>>();
+        instance.BindableIterableProperty = nestedClassList6;
     }
 
     public static void TestGenericList<T>()
@@ -434,6 +451,30 @@ sealed class NestedClass : IProperties2
     {
         private int _value;
         public int ReadWriteProperty { get => _value; set => _value = value; }
+
+        internal sealed class NestedClass2 : IProperties2
+        {
+            private int _value;
+            public int ReadWriteProperty { get => _value; set => _value = value; }
+        }
+    }
+
+    sealed class NestedGenericClass<T> : IProperties2
+    {
+        private int _value;
+        public int ReadWriteProperty { get => _value; set => _value = value; }
+
+        internal sealed class NestedClass2 : IProperties2
+        {
+            private int _value;
+            public int ReadWriteProperty { get => _value; set => _value = value; }
+        }
+
+        internal sealed class NestedClass3<S> : IProperties2
+        {
+            private int _value;
+            public int ReadWriteProperty { get => _value; set => _value = value; }
+        }
     }
 }
 

src/Tests/FunctionalTests/Collections/Program.cs

@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading;
 using test_component_derived.Nested;
 using TestComponentCSharp;
 
@@ -68,6 +69,20 @@
     return 101;
 }
 
+var cancellationDictionary = new Dictionary<string, CancellationTokenSource>();
+instance.BindableIterableProperty = cancellationDictionary;
+if (cancellationDictionary != instance.BindableIterableProperty)
+{
+    return 101;
+}
+
+var observableCollection = new System.Collections.ObjectModel.ObservableCollection<string>();
+instance.BindableIterableProperty = observableCollection;
+if (observableCollection != instance.BindableIterableProperty)
+{
+    return 101;
+}
+
 return 100;
 
 static bool SequencesEqual<T>(IEnumerable<T> x, params IEnumerable<T>[] list) => list.All((y) => x.SequenceEqual(y));

src/Tests/FunctionalTests/Events/Program.cs

@@ -1,7 +1,9 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading;
 using TestComponentCSharp;
+using Windows.Foundation.Collections;
 
 int events_expected = 0;
 int events_received = 0;
@@ -66,7 +68,26 @@
 // fired twice.
 events_expected += 3;
 
-return events_received == events_expected && uriMatches ? 100 : 101;
+ProvideInt s = () => 4;
+instance.ObjectProperty = s;
+bool boxedDelegateMatches = ((ProvideInt)instance.ObjectProperty) == s;
+
+TestDelegate t = () => true;
+instance.ObjectProperty = t;
+boxedDelegateMatches &= ((TestDelegate)instance.ObjectProperty) == t;
+
+System.EventHandler<int> u = (object sender, int args) => { };
+instance.ObjectProperty = u;
+boxedDelegateMatches &= ((System.EventHandler<int>)instance.ObjectProperty) == u;
+
+System.EventHandler<CancellationToken> v = (object sender, CancellationToken args) => { };
+instance.ObjectProperty = v;
+boxedDelegateMatches &= ((System.EventHandler<CancellationToken>)instance.ObjectProperty) == v;
+
+TestDelegate[] arr = new TestDelegate[] { t, t };
+instance.ObjectProperty = arr;
+
+return events_received == events_expected && uriMatches && boxedDelegateMatches ? 100 : 101;
 
 void Instance_Event0()
 {
@@ -91,4 +112,18 @@ public void AddUriHandler(ProvideUri provideUri)
     {
         Uri = provideUri();
     }
-}
+}
+
+partial class ObservableDictionaryChangedEventArgs : IMapChangedEventArgs<string>
+{
+    public ObservableDictionaryChangedEventArgs(CollectionChange change, string key)
+    {
+        CollectionChange = change;
+        Key = key;
+    }
+
+    public CollectionChange CollectionChange { get; private set; }
+    public string Key { get; private set; }
+}
+
+delegate bool TestDelegate();