Skip to content

Commit

Permalink
Fix scenario where source generator isn't detecting parameters passed…
Browse files Browse the repository at this point in the history
… to WinRT mapped interfaces (#1867)

* Fix scenario where source generator isn't detecting parameters passed to WinRT mapped interfaces

* Add another test case
  • Loading branch information
manodasanW authored Nov 11, 2024
1 parent 58c60a9 commit 8649ee3
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 24 deletions.
22 changes: 11 additions & 11 deletions src/Authoring/WinRT.SourceGenerator/AotOptimizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,7 @@ private static (VtableAttribute, EquatableArray<VtableAttribute>) 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)
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1176,20 +1174,21 @@ private static EquatableArray<VtableAttribute> 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<VtableAttribute> GetVtableAttributesToAddOnLookupTable(
GeneratorSyntaxContext context,
TypeMapper typeMapper,
Func<ISymbol, bool> isManagedOnlyType,
Func<ISymbol, TypeMapper, bool> isWinRTType,
Func<ISymbol, bool> isWinRTClass)
Func<ISymbol, bool, bool> isWinRTClassOrInterface)
{
HashSet<ITypeSymbol> visitedTypes = new(SymbolEqualityComparer.Default);
HashSet<VtableAttribute> vtableAttributes = new();
Expand All @@ -1203,7 +1202,7 @@ private static EquatableArray<VtableAttribute> 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
Expand Down Expand Up @@ -1232,13 +1231,14 @@ private static EquatableArray<VtableAttribute> 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);
Expand Down Expand Up @@ -1423,7 +1423,7 @@ private static EquatableArray<VtableAttribute> 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<VtableAttribute> vtableAttributes = new();

foreach (var attributeData in context.Attributes)
Expand Down
22 changes: 14 additions & 8 deletions src/Authoring/WinRT.SourceGenerator/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ISymbol, bool> 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<ISymbol, bool, bool> IsWinRTClassOrInterface(Compilation compilation, Func<ISymbol, TypeMapper, bool> 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);
}
}
Expand Down Expand Up @@ -668,14 +674,14 @@ public static bool IsManagedOnlyType(ISymbol symbol, ITypeSymbol winrtExposedTyp
return false;
}

public static Func<ISymbol, TypeMapper, bool> IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(Compilation compilation)
public static Func<ISymbol, TypeMapper, bool> 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);
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/Authoring/WinRT.SourceGenerator/WinRTAotCodeFixer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
20 changes: 20 additions & 0 deletions src/Tests/FunctionalTests/Collections/Program.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel;
using System.Linq;
using System.Threading;
Expand Down Expand Up @@ -268,6 +269,25 @@
Action<int, int> s = (a, b) => { _ = a + b; };
ActionToFunction(s)(2, 3);

var intToListDict = instance.GetIntToListDictionary();
intToListDict.Add(2, new List<EnumValue> { EnumValue.One, EnumValue.Two });
intToListDict[4] = new Collection<EnumValue> { 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<T>(IEnumerable<T> x, params IEnumerable<T>[] list) => list.All((y) => x.SequenceEqual(y));
Expand Down
6 changes: 6 additions & 0 deletions src/Tests/TestComponentCSharp/Class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,12 @@ namespace winrt::TestComponentCSharp::implementation
{ L"String2", ComposedNonBlittableStruct{ { 2 }, { L"String2" }, { true, false, true, false }, { 2 } } }
});
}

IMap<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>> Class::GetIntToListDictionary()
{
auto vector = winrt::single_threaded_vector(std::vector { TestComponentCSharp::EnumValue::One });
return single_threaded_map<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>>(std::map<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>>{ {1, vector}});
}

struct ComposedBlittableStructComparer
{
Expand Down
3 changes: 2 additions & 1 deletion src/Tests/TestComponentCSharp/Class.h
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,8 @@ namespace winrt::TestComponentCSharp::implementation
Windows::Foundation::Collections::IMap<hstring, TestComponentCSharp::ComposedBlittableStruct> GetStringToBlittableDictionary();
Windows::Foundation::Collections::IMap<hstring, TestComponentCSharp::ComposedNonBlittableStruct> GetStringToNonBlittableDictionary();
Windows::Foundation::Collections::IMap<TestComponentCSharp::ComposedBlittableStruct, Windows::Foundation::IInspectable> GetBlittableToObjectDictionary();

Windows::Foundation::Collections::IMap<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>> GetIntToListDictionary();

// Test IIDOptimizer -- testing the windows projection covers most code paths, and these two types exercise the rest.
Windows::Foundation::Collections::IVectorView<Microsoft::UI::Xaml::Data::DataErrorsChangedEventArgs> GetEventArgsVector();
Windows::Foundation::Collections::IVectorView<TestComponentCSharp::ProvideUri> GetNonGenericDelegateVector();
Expand Down
1 change: 1 addition & 0 deletions src/Tests/TestComponentCSharp/TestComponentCSharp.idl
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ namespace TestComponentCSharp
Windows.Foundation.Collections.IMap<String, ComposedBlittableStruct> GetStringToBlittableDictionary();
Windows.Foundation.Collections.IMap<String, ComposedNonBlittableStruct> GetStringToNonBlittableDictionary();
Windows.Foundation.Collections.IMap<ComposedBlittableStruct, Object> GetBlittableToObjectDictionary();
Windows.Foundation.Collections.IMap<Int32, Windows.Foundation.Collections.IVector<EnumValue> > GetIntToListDictionary();

// Test IIDOptimizer
Windows.Foundation.Collections.IVectorView<Microsoft.UI.Xaml.Data.DataErrorsChangedEventArgs> GetEventArgsVector();
Expand Down

0 comments on commit 8649ee3

Please sign in to comment.