From 8649ee3eeb2445ca2a36d80d878ef60b96a6c65d Mon Sep 17 00:00:00 2001 From: Manodasan Wignarajah Date: Sun, 10 Nov 2024 16:54:30 -0800 Subject: [PATCH] Fix scenario where source generator isn't detecting parameters passed to WinRT mapped interfaces (#1867) * Fix scenario where source generator isn't detecting parameters passed to WinRT mapped interfaces * Add another test case --- .../WinRT.SourceGenerator/AotOptimizer.cs | 22 +++++++++---------- src/Authoring/WinRT.SourceGenerator/Helper.cs | 22 ++++++++++++------- .../WinRTAotCodeFixer.cs | 10 +++++---- .../FunctionalTests/Collections/Program.cs | 20 +++++++++++++++++ src/Tests/TestComponentCSharp/Class.cpp | 6 +++++ src/Tests/TestComponentCSharp/Class.h | 3 ++- .../TestComponentCSharp.idl | 1 + 7 files changed, 60 insertions(+), 24 deletions(-) diff --git a/src/Authoring/WinRT.SourceGenerator/AotOptimizer.cs b/src/Authoring/WinRT.SourceGenerator/AotOptimizer.cs index 5a8b0251e..7a4a0b4bd 100644 --- a/src/Authoring/WinRT.SourceGenerator/AotOptimizer.cs +++ b/src/Authoring/WinRT.SourceGenerator/AotOptimizer.cs @@ -239,9 +239,7 @@ private static (VtableAttribute, EquatableArray) GetVtableAttri bool isCsWinRTCcwLookupTableGeneratorEnabled) { var isManagedOnlyType = GeneratorHelper.IsManagedOnlyType(compilation); - var isWinRTTypeFunc = checkForComponentTypes ? - GeneratorHelper.IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(compilation) : - GeneratorHelper.IsWinRTType; + var isWinRTTypeFunc = GeneratorHelper.IsWinRTType(compilation, checkForComponentTypes); var vtableAttribute = GetVtableAttributeToAdd(symbol, isManagedOnlyType, isWinRTTypeFunc, typeMapper, compilation, false); if (vtableAttribute != default) { @@ -317,7 +315,7 @@ private static VtableAttribute GetVtableAttributesForTaskAdapters(GeneratorSynta return GetVtableAttributeToAdd( constructedAdapterType, GeneratorHelper.IsManagedOnlyType(context.SemanticModel.Compilation), - !isCsWinRTComponent ? GeneratorHelper.IsWinRTType : GeneratorHelper.IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(context.SemanticModel.Compilation), + GeneratorHelper.IsWinRTType(context.SemanticModel.Compilation, isCsWinRTComponent), typeMapper, context.SemanticModel.Compilation, false); @@ -1176,12 +1174,13 @@ private static EquatableArray GetVtableAttributesToAddOnLookupT TypeMapper typeMapper, bool isCsWinRTComponent) { + var isWinRTType = GeneratorHelper.IsWinRTType(context.SemanticModel.Compilation, isCsWinRTComponent); return GetVtableAttributesToAddOnLookupTable( context, typeMapper, GeneratorHelper.IsManagedOnlyType(context.SemanticModel.Compilation), - !isCsWinRTComponent ? GeneratorHelper.IsWinRTType : GeneratorHelper.IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(context.SemanticModel.Compilation), - GeneratorHelper.IsWinRTClass(context.SemanticModel.Compilation)); + isWinRTType, + GeneratorHelper.IsWinRTClassOrInterface(context.SemanticModel.Compilation, isWinRTType, typeMapper)); } private static EquatableArray GetVtableAttributesToAddOnLookupTable( @@ -1189,7 +1188,7 @@ private static EquatableArray GetVtableAttributesToAddOnLookupT TypeMapper typeMapper, Func isManagedOnlyType, Func isWinRTType, - Func isWinRTClass) + Func isWinRTClassOrInterface) { HashSet visitedTypes = new(SymbolEqualityComparer.Default); HashSet vtableAttributes = new(); @@ -1203,7 +1202,7 @@ private static EquatableArray GetVtableAttributesToAddOnLookupT // and end up calling a projection function (i.e. ones generated by XAML compiler) // In theory, another library can also be called which can call a projected function // but not handling those scenarios for now. - (isWinRTClass(methodSymbol.ContainingSymbol) || + (isWinRTClassOrInterface(methodSymbol.ContainingSymbol, true) || SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly))) { // Get the concrete types directly from the argument rather than @@ -1232,13 +1231,14 @@ private static EquatableArray GetVtableAttributesToAddOnLookupT { var leftSymbol = context.SemanticModel.GetSymbolInfo(assignment.Left).Symbol; if (leftSymbol is IPropertySymbol propertySymbol && - (isWinRTClass(propertySymbol.ContainingSymbol) || + (isWinRTClassOrInterface(propertySymbol.ContainingSymbol, true) || SymbolEqualityComparer.Default.Equals(propertySymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly))) { AddVtableAttributesForType(context.SemanticModel.GetTypeInfo(assignment.Right), propertySymbol.Type); } else if (leftSymbol is IFieldSymbol fieldSymbol && - (isWinRTClass(fieldSymbol.ContainingSymbol) || + // WinRT interfaces don't have fields, so we don't need to check for them. + (isWinRTClassOrInterface(fieldSymbol.ContainingSymbol, false) || SymbolEqualityComparer.Default.Equals(fieldSymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly))) { AddVtableAttributesForType(context.SemanticModel.GetTypeInfo(assignment.Right), fieldSymbol.Type); @@ -1423,7 +1423,7 @@ private static EquatableArray GetVtableAttributesToAddOnLookupT bool isCsWinRTComponent) { var isManagedOnlyType = GeneratorHelper.IsManagedOnlyType(context.SemanticModel.Compilation); - var isWinRTType = !isCsWinRTComponent ? GeneratorHelper.IsWinRTType : GeneratorHelper.IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(context.SemanticModel.Compilation); + var isWinRTType = GeneratorHelper.IsWinRTType(context.SemanticModel.Compilation, isCsWinRTComponent); HashSet vtableAttributes = new(); foreach (var attributeData in context.Attributes) diff --git a/src/Authoring/WinRT.SourceGenerator/Helper.cs b/src/Authoring/WinRT.SourceGenerator/Helper.cs index 39c21fb19..83f5e67cf 100644 --- a/src/Authoring/WinRT.SourceGenerator/Helper.cs +++ b/src/Authoring/WinRT.SourceGenerator/Helper.cs @@ -368,16 +368,22 @@ public static bool AllowUnsafe(Compilation compilation) return compilation is CSharpCompilation csharpCompilation && csharpCompilation.Options.AllowUnsafe; } - // Whether the class itself is a WinRT projected class. - // This is similar to whether it is a WinRT type, but custom type mappings - // are excluded given those are C# implemented classes. - public static Func IsWinRTClass(Compilation compilation) + // Returns whether it is a WinRT class or interface. + // If the bool parameter is true, then custom mapped interfaces are also considered. + // This function is similar to whether it is a WinRT type, but custom type mapped + // classes are excluded given those are C# implemented classes such as string. + public static Func IsWinRTClassOrInterface(Compilation compilation, Func isWinRTType, TypeMapper mapper) { var winrtRuntimeTypeAttribute = compilation.GetTypeByMetadataName("WinRT.WindowsRuntimeTypeAttribute"); - return IsWinRTClassHelper; + return IsWinRTClassOrInterfaceHelper; - bool IsWinRTClassHelper(ISymbol type) + bool IsWinRTClassOrInterfaceHelper(ISymbol type, bool includeMappedInterfaces) { + if (type is ITypeSymbol typeSymbol && typeSymbol.TypeKind == TypeKind.Interface) + { + return isWinRTType(type, mapper); + } + return HasAttributeWithType(type, winrtRuntimeTypeAttribute); } } @@ -668,14 +674,14 @@ public static bool IsManagedOnlyType(ISymbol symbol, ITypeSymbol winrtExposedTyp return false; } - public static Func IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(Compilation compilation) + public static Func IsWinRTType(Compilation compilation, bool isComponentProject) { var winrtTypeAttribute = compilation.GetTypeByMetadataName("WinRT.WindowsRuntimeTypeAttribute"); return IsWinRTTypeHelper; bool IsWinRTTypeHelper(ISymbol type, TypeMapper typeMapper) { - return IsWinRTType(type, winrtTypeAttribute, typeMapper, true, compilation.Assembly); + return IsWinRTType(type, winrtTypeAttribute, typeMapper, isComponentProject, compilation.Assembly); } } diff --git a/src/Authoring/WinRT.SourceGenerator/WinRTAotCodeFixer.cs b/src/Authoring/WinRT.SourceGenerator/WinRTAotCodeFixer.cs index 6a9eaf512..eab785975 100644 --- a/src/Authoring/WinRT.SourceGenerator/WinRTAotCodeFixer.cs +++ b/src/Authoring/WinRT.SourceGenerator/WinRTAotCodeFixer.cs @@ -146,7 +146,8 @@ public override void Initialize(AnalysisContext context) { context.RegisterSyntaxNodeAction(context => { - var isWinRTClass = GeneratorHelper.IsWinRTClass(context.SemanticModel.Compilation); + var isWinRTType = GeneratorHelper.IsWinRTType(context.SemanticModel.Compilation, isComponentProject); + var isWinRTClassOrInterface = GeneratorHelper.IsWinRTClassOrInterface(context.SemanticModel.Compilation, isWinRTType, typeMapper); if (context.Node is InvocationExpressionSyntax invocation) { @@ -164,7 +165,7 @@ public override void Initialize(AnalysisContext context) // and end up calling a projection function (i.e. ones generated by XAML compiler) // In theory, another library can also be called which can call a projected function // but not handling those scenarios for now. - else if(isWinRTClass(methodSymbol.ContainingSymbol) || + else if(isWinRTClassOrInterface(methodSymbol.ContainingSymbol, true) || SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly)) { // Get the concrete types directly from the argument rather than @@ -198,7 +199,7 @@ public override void Initialize(AnalysisContext context) { var leftSymbol = context.SemanticModel.GetSymbolInfo(assignment.Left).Symbol; if (leftSymbol is IPropertySymbol propertySymbol && - (isWinRTClass(propertySymbol.ContainingSymbol) || + (isWinRTClassOrInterface(propertySymbol.ContainingSymbol, true) || SymbolEqualityComparer.Default.Equals(propertySymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly))) { var assignmentType = context.SemanticModel.GetTypeInfo(assignment.Right); @@ -209,7 +210,8 @@ public override void Initialize(AnalysisContext context) } } else if (leftSymbol is IFieldSymbol fieldSymbol && - (isWinRTClass(fieldSymbol.ContainingSymbol) || + // WinRT interfaces don't have fields, so we don't need to check for them. + (isWinRTClassOrInterface(fieldSymbol.ContainingSymbol, false) || SymbolEqualityComparer.Default.Equals(fieldSymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly))) { var assignmentType = context.SemanticModel.GetTypeInfo(assignment.Right); diff --git a/src/Tests/FunctionalTests/Collections/Program.cs b/src/Tests/FunctionalTests/Collections/Program.cs index 3adab7b20..d5967dc1c 100644 --- a/src/Tests/FunctionalTests/Collections/Program.cs +++ b/src/Tests/FunctionalTests/Collections/Program.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.ComponentModel; using System.Linq; using System.Threading; @@ -268,6 +269,25 @@ Action s = (a, b) => { _ = a + b; }; ActionToFunction(s)(2, 3); +var intToListDict = instance.GetIntToListDictionary(); +intToListDict.Add(2, new List { EnumValue.One, EnumValue.Two }); +intToListDict[4] = new Collection { EnumValue.Two }; +if (intToListDict.Count != 3 || + intToListDict[1].First() != EnumValue.One) +{ + return 101; +} + +if (intToListDict[2].Count != 2 || intToListDict[2].First() != EnumValue.One) +{ + return 101; +} + +if (intToListDict[4].Count != 1 || intToListDict[4].First() != EnumValue.Two) +{ + return 101; +} + return 100; static bool SequencesEqual(IEnumerable x, params IEnumerable[] list) => list.All((y) => x.SequenceEqual(y)); diff --git a/src/Tests/TestComponentCSharp/Class.cpp b/src/Tests/TestComponentCSharp/Class.cpp index 044fa3f87..19768f8f3 100644 --- a/src/Tests/TestComponentCSharp/Class.cpp +++ b/src/Tests/TestComponentCSharp/Class.cpp @@ -1097,6 +1097,12 @@ namespace winrt::TestComponentCSharp::implementation { L"String2", ComposedNonBlittableStruct{ { 2 }, { L"String2" }, { true, false, true, false }, { 2 } } } }); } + + IMap> Class::GetIntToListDictionary() + { + auto vector = winrt::single_threaded_vector(std::vector { TestComponentCSharp::EnumValue::One }); + return single_threaded_map>(std::map>{ {1, vector}}); + } struct ComposedBlittableStructComparer { diff --git a/src/Tests/TestComponentCSharp/Class.h b/src/Tests/TestComponentCSharp/Class.h index 240752f4c..c3dfb8266 100644 --- a/src/Tests/TestComponentCSharp/Class.h +++ b/src/Tests/TestComponentCSharp/Class.h @@ -291,7 +291,8 @@ namespace winrt::TestComponentCSharp::implementation Windows::Foundation::Collections::IMap GetStringToBlittableDictionary(); Windows::Foundation::Collections::IMap GetStringToNonBlittableDictionary(); Windows::Foundation::Collections::IMap GetBlittableToObjectDictionary(); - + Windows::Foundation::Collections::IMap> GetIntToListDictionary(); + // Test IIDOptimizer -- testing the windows projection covers most code paths, and these two types exercise the rest. Windows::Foundation::Collections::IVectorView GetEventArgsVector(); Windows::Foundation::Collections::IVectorView GetNonGenericDelegateVector(); diff --git a/src/Tests/TestComponentCSharp/TestComponentCSharp.idl b/src/Tests/TestComponentCSharp/TestComponentCSharp.idl index 6a0b33749..0d3259fd5 100644 --- a/src/Tests/TestComponentCSharp/TestComponentCSharp.idl +++ b/src/Tests/TestComponentCSharp/TestComponentCSharp.idl @@ -338,6 +338,7 @@ namespace TestComponentCSharp Windows.Foundation.Collections.IMap GetStringToBlittableDictionary(); Windows.Foundation.Collections.IMap GetStringToNonBlittableDictionary(); Windows.Foundation.Collections.IMap GetBlittableToObjectDictionary(); + Windows.Foundation.Collections.IMap > GetIntToListDictionary(); // Test IIDOptimizer Windows.Foundation.Collections.IVectorView GetEventArgsVector();