From a7e960e3d38a8e1222145bd707909d40a60443e2 Mon Sep 17 00:00:00 2001 From: Manodasan Wignarajah Date: Tue, 8 Oct 2024 23:56:23 -0700 Subject: [PATCH 1/4] Detect if a type is defined in mutliple places so that we can avoid a compiler error in the fallback generator --- .../CsWinRTDiagnosticStrings.Designer.cs | 9 +++ .../CsWinRTDiagnosticStrings.resx | 3 + .../RcwReflectionFallbackGenerator.cs | 62 ++++++++++++++----- .../WinRT.SourceGenerator/WinRTRules.cs | 13 ++++ 4 files changed, 71 insertions(+), 16 deletions(-) diff --git a/src/Authoring/WinRT.SourceGenerator/CsWinRTDiagnosticStrings.Designer.cs b/src/Authoring/WinRT.SourceGenerator/CsWinRTDiagnosticStrings.Designer.cs index ddfce497f..65d770159 100644 --- a/src/Authoring/WinRT.SourceGenerator/CsWinRTDiagnosticStrings.Designer.cs +++ b/src/Authoring/WinRT.SourceGenerator/CsWinRTDiagnosticStrings.Designer.cs @@ -240,6 +240,15 @@ internal static string ClassNotMarkedPartial_Text { } } + /// + /// Looks up a localized string similar to Class '{0}' was generated using an older version of CsWinRT and due to the type being defined in multiple DLLs, CsWinRT can not generate compat code to make it trimming safe. Update to a projection generated using CsWinRT 2.1.0 or later for trimming and AOT compatibility.. + /// + internal static string ClassOldProjectionMultipleInstances_Text { + get { + return ResourceManager.GetString("ClassOldProjectionMultipleInstances_Text", resourceCulture); + } + } + /// /// Looks up a localized string similar to Namespace is disjoint from main (winmd) namespace. /// diff --git a/src/Authoring/WinRT.SourceGenerator/CsWinRTDiagnosticStrings.resx b/src/Authoring/WinRT.SourceGenerator/CsWinRTDiagnosticStrings.resx index 6f0ffe13c..6c1d7f141 100644 --- a/src/Authoring/WinRT.SourceGenerator/CsWinRTDiagnosticStrings.resx +++ b/src/Authoring/WinRT.SourceGenerator/CsWinRTDiagnosticStrings.resx @@ -109,6 +109,9 @@ Class '{0}' implements WinRT interface(s) {1} generated using an older version of CsWinRT. Update to a projection generated using CsWinRT 2.1.0 or later for trimming and AOT compatibility. + + Class '{0}' was generated using an older version of CsWinRT and due to the type being defined in multiple DLLs, CsWinRT can not generate compat code to make it trimming safe. Update to a projection generated using CsWinRT 2.1.0 or later for trimming and AOT compatibility. + Class is not marked partial diff --git a/src/Authoring/WinRT.SourceGenerator/RcwReflectionFallbackGenerator.cs b/src/Authoring/WinRT.SourceGenerator/RcwReflectionFallbackGenerator.cs index 35e9a9033..c1fd75dee 100644 --- a/src/Authoring/WinRT.SourceGenerator/RcwReflectionFallbackGenerator.cs +++ b/src/Authoring/WinRT.SourceGenerator/RcwReflectionFallbackGenerator.cs @@ -56,6 +56,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context) IncrementalValueProvider isGeneratorForceOptOut = context.AnalyzerConfigOptionsProvider.Select(static (options, token) => { return options.GetCsWinRTRcwFactoryFallbackGeneratorForceOptOut(); + }); + + IncrementalValueProvider csWinRTAotWarningEnabled = context.AnalyzerConfigOptionsProvider.Select(static (options, token) => + { + return options.GetCsWinRTAotWarningLevel() >= 1; }); // Get whether the generator should actually run or not @@ -72,20 +77,20 @@ public void Initialize(IncrementalGeneratorInitializationContext context) .Where(static item => item.Right); // Get all the names of the projected types to root - IncrementalValuesProvider> executableTypeNames = enabledExecutableReferences.Select(static (executableReference, token) => + IncrementalValuesProvider> executableTypeNames = enabledExecutableReferences.Select(static (executableReference, token) => { Compilation compilation = executableReference.Value.GetCompilationUnsafe(); // We only care about resolved assembly symbols (this should always be the case anyway) if (compilation.GetAssemblyOrModuleSymbol(executableReference.Value.Reference) is not IAssemblySymbol assemblySymbol) { - return EquatableArray.FromImmutableArray(ImmutableArray.Empty); + return EquatableArray.FromImmutableArray(ImmutableArray.Empty); } // If the assembly is not an old projections assembly, we have nothing to do if (!GeneratorHelper.IsOldProjectionAssembly(assemblySymbol)) { - return EquatableArray.FromImmutableArray(ImmutableArray.Empty); + return EquatableArray.FromImmutableArray(ImmutableArray.Empty); } token.ThrowIfCancellationRequested(); @@ -93,7 +98,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) ITypeSymbol attributeSymbol = compilation.GetTypeByMetadataName("System.Attribute")!; ITypeSymbol windowsRuntimeTypeAttributeSymbol = compilation.GetTypeByMetadataName("WinRT.WindowsRuntimeTypeAttribute")!; - ImmutableArray.Builder executableTypeNames = ImmutableArray.CreateBuilder(); + ImmutableArray.Builder executableTypeNames = ImmutableArray.CreateBuilder(); // Process all type symbols in the current assembly foreach (INamedTypeSymbol typeSymbol in VisitNamedTypeSymbolsExceptABI(assemblySymbol)) @@ -141,25 +146,31 @@ public void Initialize(IncrementalGeneratorInitializationContext context) continue; } - executableTypeNames.Add(typeName); + // Check if we are able to resolve the type using GetTypeByMetadataName. If not, + // it typically indicates there are multiple definitions of this type in the references + // and us emitting a dependency on this type would cause compiler error. So emit + // a warning instead. + bool hasMultipleDefinitions = compilation.GetTypeByMetadataName(typeSymbol.MetadataName) is null; + executableTypeNames.Add(new RcwReflectionFallbackType(typeName, hasMultipleDefinitions)); } token.ThrowIfCancellationRequested(); - return EquatableArray.FromImmutableArray(executableTypeNames.ToImmutable()); + return EquatableArray.FromImmutableArray(executableTypeNames.ToImmutable()); }); // Combine all names into a single sequence - IncrementalValueProvider> projectedTypeNames = + IncrementalValueProvider<(ImmutableArray, bool)> projectedTypeNamesAndAotWarningEnabled = executableTypeNames .Where(static names => !names.IsEmpty) .SelectMany(static (executableTypeNames, token) => executableTypeNames.AsImmutableArray()) - .Collect(); + .Collect() + .Combine(csWinRTAotWarningEnabled); // Generate the [DynamicDependency] attributes - context.RegisterImplementationSourceOutput(projectedTypeNames, static (context, projectedTypeNames) => + context.RegisterImplementationSourceOutput(projectedTypeNamesAndAotWarningEnabled, static (SourceProductionContext context, (ImmutableArray projectedTypeNames, bool csWinRTAotWarningEnabled) value) => { - if (projectedTypeNames.IsEmpty) + if (value.projectedTypeNames.IsEmpty) { return; } @@ -187,11 +198,25 @@ internal static class RcwFallbackInitializer [ModuleInitializer] """); - foreach (string projectedTypeName in projectedTypeNames) + bool emittedDynamicDependency = false; + foreach (RcwReflectionFallbackType projectedTypeName in value.projectedTypeNames) { - builder.Append(" [DynamicDependency(DynamicallyAccessedMemberTypes.NonPublicConstructors, typeof("); - builder.Append(projectedTypeName); - builder.AppendLine("))]"); + // If there are multiple definitions of the type, emitting a dependency would result in a compiler error. + // So instead, emit a diagnostic for it. + if (projectedTypeName.HasMultipleDefinitions) + { + var diagnosticDescriptor = value.csWinRTAotWarningEnabled ? + WinRTRules.ClassNotAotCompatibleOldProjectionMultipleInstancesWarning : WinRTRules.ClassNotAotCompatibleOldProjectionMultipleInstancesInfo; + // We have no location to emit the diagnostic as this is just a reference we detect. + context.ReportDiagnostic(Diagnostic.Create(diagnosticDescriptor, null, projectedTypeName.TypeName)); + } + else + { + emittedDynamicDependency = true; + builder.Append(" [DynamicDependency(DynamicallyAccessedMemberTypes.NonPublicConstructors, typeof("); + builder.Append(projectedTypeName.TypeName); + builder.AppendLine("))]"); + } } builder.Append(""" @@ -202,7 +227,10 @@ public static void InitializeRcwFallback() } """); - context.AddSource("RcwFallbackInitializer.g.cs", builder.ToString()); + if (emittedDynamicDependency) + { + context.AddSource("RcwFallbackInitializer.g.cs", builder.ToString()); + } }); } @@ -305,5 +333,7 @@ public bool Equals(EquatablePortableExecutableReference other) return other.Reference.GetMetadataId() == Reference.GetMetadataId(); } - } + } + + internal readonly record struct RcwReflectionFallbackType(string TypeName, bool HasMultipleDefinitions); } diff --git a/src/Authoring/WinRT.SourceGenerator/WinRTRules.cs b/src/Authoring/WinRT.SourceGenerator/WinRTRules.cs index d244c9d8c..5ab4a338f 100644 --- a/src/Authoring/WinRT.SourceGenerator/WinRTRules.cs +++ b/src/Authoring/WinRT.SourceGenerator/WinRTRules.cs @@ -225,5 +225,18 @@ private static DiagnosticDescriptor MakeRule(string id, string title, string mes CsWinRTDiagnosticStrings.BindableCustomPropertyClassNotMarkedPartial_Text, false, true); + + public static DiagnosticDescriptor ClassNotAotCompatibleOldProjectionMultipleInstancesWarning = MakeRule( + "CsWinRT1029", + CsWinRTDiagnosticStrings.ClassImplementsOldProjection_Brief, + CsWinRTDiagnosticStrings.ClassOldProjectionMultipleInstances_Text, + false, + true); + + public static DiagnosticDescriptor ClassNotAotCompatibleOldProjectionMultipleInstancesInfo = MakeRule( + "CsWinRT1029", + CsWinRTDiagnosticStrings.ClassImplementsOldProjection_Brief, + CsWinRTDiagnosticStrings.ClassOldProjectionMultipleInstances_Text, + false); } } From 49fe657374b7e0a8c51ac38c5bc63b6e0c44ae95 Mon Sep 17 00:00:00 2001 From: Manodasan Wignarajah Date: Wed, 9 Oct 2024 00:53:02 -0700 Subject: [PATCH 2/4] Fix NotifyCollectionChangedAction not marked as blittable or a value type --- src/Authoring/WinRT.SourceGenerator/TypeMapper.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Authoring/WinRT.SourceGenerator/TypeMapper.cs b/src/Authoring/WinRT.SourceGenerator/TypeMapper.cs index 1c9c9a3a3..b591e76b0 100644 --- a/src/Authoring/WinRT.SourceGenerator/TypeMapper.cs +++ b/src/Authoring/WinRT.SourceGenerator/TypeMapper.cs @@ -45,7 +45,7 @@ public TypeMapper(bool useWindowsUIXamlProjections) { "System.Collections.IEnumerable", new MappedType("Windows.UI.Xaml.Interop", "IBindableIterable", "Windows.UI.Xaml") }, { "System.Collections.IList", new MappedType("Windows.UI.Xaml.Interop", "IBindableVector", "Windows.UI.Xaml") }, { "System.Collections.Specialized.INotifyCollectionChanged", new MappedType("Windows.UI.Xaml.Interop", "INotifyCollectionChanged", "Windows.UI.Xaml") }, - { "System.Collections.Specialized.NotifyCollectionChangedAction", new MappedType("Windows.UI.Xaml.Interop", "NotifyCollectionChangedAction", "Windows.UI.Xaml") }, + { "System.Collections.Specialized.NotifyCollectionChangedAction", new MappedType("Windows.UI.Xaml.Interop", "NotifyCollectionChangedAction", "Windows.UI.Xaml", true, true) }, { "System.Collections.Specialized.NotifyCollectionChangedEventArgs", new MappedType("Windows.UI.Xaml.Interop", "NotifyCollectionChangedEventArgs", "Windows.UI.Xaml") }, { "System.Collections.Specialized.NotifyCollectionChangedEventHandler", new MappedType("Windows.UI.Xaml.Interop", "NotifyCollectionChangedEventHandler", "Windows.UI.Xaml") }, { "WinRT.EventRegistrationToken", new MappedType("Windows.Foundation", "EventRegistrationToken", "Windows.Foundation.FoundationContract", true, true) }, @@ -92,7 +92,7 @@ public TypeMapper(bool useWindowsUIXamlProjections) { "System.Collections.IEnumerable", new MappedType("Microsoft.UI.Xaml.Interop", "IBindableIterable", "Microsoft.UI") }, { "System.Collections.IList", new MappedType("Microsoft.UI.Xaml.Interop", "IBindableVector", "Microsoft.UI") }, { "System.Collections.Specialized.INotifyCollectionChanged", new MappedType("Microsoft.UI.Xaml.Interop", "INotifyCollectionChanged", "Microsoft.UI") }, - { "System.Collections.Specialized.NotifyCollectionChangedAction", new MappedType("Microsoft.UI.Xaml.Interop", "NotifyCollectionChangedAction", "Microsoft.UI") }, + { "System.Collections.Specialized.NotifyCollectionChangedAction", new MappedType("Microsoft.UI.Xaml.Interop", "NotifyCollectionChangedAction", "Microsoft.UI", true, true) }, { "System.Collections.Specialized.NotifyCollectionChangedEventArgs", new MappedType("Microsoft.UI.Xaml.Interop", "NotifyCollectionChangedEventArgs", "Microsoft.UI") }, { "System.Collections.Specialized.NotifyCollectionChangedEventHandler", new MappedType("Microsoft.UI.Xaml.Interop", "NotifyCollectionChangedEventHandler", "Microsoft.UI") }, { "WinRT.EventRegistrationToken", new MappedType("Windows.Foundation", "EventRegistrationToken", "Windows.Foundation.FoundationContract", true, true) }, From 6b20f4972ca0a3002f9783fd3f53f2572dd22d53 Mon Sep 17 00:00:00 2001 From: Manodasan Wignarajah Date: Wed, 9 Oct 2024 16:57:43 -0700 Subject: [PATCH 3/4] Fix RCW fallback source generator when there are multiple definitions for the same type in references. --- src/Authoring/WinRT.SourceGenerator/Helper.cs | 5 +++++ .../WinRT.SourceGenerator/RcwReflectionFallbackGenerator.cs | 6 +++--- src/Tests/FunctionalTests/CCW/Program.cs | 6 +++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Authoring/WinRT.SourceGenerator/Helper.cs b/src/Authoring/WinRT.SourceGenerator/Helper.cs index 397456121..a134a02ac 100644 --- a/src/Authoring/WinRT.SourceGenerator/Helper.cs +++ b/src/Authoring/WinRT.SourceGenerator/Helper.cs @@ -1122,5 +1122,10 @@ public static string GetTaskAdapterIfAsyncMethod(IMethodSymbol symbol) return null; } + + public static string TrimGlobalFromTypeName(string typeName) + { + return typeName.StartsWith("global::") ? typeName[8..] : typeName; + } } } \ No newline at end of file diff --git a/src/Authoring/WinRT.SourceGenerator/RcwReflectionFallbackGenerator.cs b/src/Authoring/WinRT.SourceGenerator/RcwReflectionFallbackGenerator.cs index c1fd75dee..e0360602e 100644 --- a/src/Authoring/WinRT.SourceGenerator/RcwReflectionFallbackGenerator.cs +++ b/src/Authoring/WinRT.SourceGenerator/RcwReflectionFallbackGenerator.cs @@ -147,10 +147,10 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } // Check if we are able to resolve the type using GetTypeByMetadataName. If not, - // it typically indicates there are multiple definitions of this type in the references + // it indicates there are multiple definitions of this type in the references // and us emitting a dependency on this type would cause compiler error. So emit // a warning instead. - bool hasMultipleDefinitions = compilation.GetTypeByMetadataName(typeSymbol.MetadataName) is null; + bool hasMultipleDefinitions = compilation.GetTypeByMetadataName(GeneratorHelper.TrimGlobalFromTypeName(typeName)) is null; executableTypeNames.Add(new RcwReflectionFallbackType(typeName, hasMultipleDefinitions)); } @@ -208,7 +208,7 @@ internal static class RcwFallbackInitializer var diagnosticDescriptor = value.csWinRTAotWarningEnabled ? WinRTRules.ClassNotAotCompatibleOldProjectionMultipleInstancesWarning : WinRTRules.ClassNotAotCompatibleOldProjectionMultipleInstancesInfo; // We have no location to emit the diagnostic as this is just a reference we detect. - context.ReportDiagnostic(Diagnostic.Create(diagnosticDescriptor, null, projectedTypeName.TypeName)); + context.ReportDiagnostic(Diagnostic.Create(diagnosticDescriptor, null, GeneratorHelper.TrimGlobalFromTypeName(projectedTypeName.TypeName))); } else { diff --git a/src/Tests/FunctionalTests/CCW/Program.cs b/src/Tests/FunctionalTests/CCW/Program.cs index f50015572..660d4ee62 100644 --- a/src/Tests/FunctionalTests/CCW/Program.cs +++ b/src/Tests/FunctionalTests/CCW/Program.cs @@ -7,6 +7,7 @@ using System.Runtime.CompilerServices; using System.Windows.Input; using System.Runtime.InteropServices; +using System.Collections.Specialized; var managedProperties = new ManagedProperties(42); var instance = new Class(); @@ -174,6 +175,9 @@ var managedWarningClassList = new List(); instance.BindableIterableProperty = managedWarningClassList; +var notifyCollectionChangedActionList = new List(); +instance.BindableIterableProperty = notifyCollectionChangedActionList; + var customCommand = new CustomCommand() as ICommand; ccw = MarshalInspectable.CreateMarshaler(customCommand); ccw.TryAs(ABI.System.Windows.Input.ICommandMethods.IID, out var commandCCW); @@ -738,7 +742,7 @@ namespace Test { namespace Test2 { - sealed partial class Nested + sealed partial class Nested2 { [GeneratedBindableCustomProperty([nameof(Value)], [])] sealed partial class Language3 : IProperties2 From bd96a426d05470186d8a17a40d07570166ba3bc9 Mon Sep 17 00:00:00 2001 From: Manodasan Wignarajah Date: Wed, 9 Oct 2024 17:32:35 -0700 Subject: [PATCH 4/4] Undo rename --- src/Tests/FunctionalTests/CCW/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tests/FunctionalTests/CCW/Program.cs b/src/Tests/FunctionalTests/CCW/Program.cs index 660d4ee62..8d709df53 100644 --- a/src/Tests/FunctionalTests/CCW/Program.cs +++ b/src/Tests/FunctionalTests/CCW/Program.cs @@ -742,7 +742,7 @@ namespace Test { namespace Test2 { - sealed partial class Nested2 + sealed partial class Nested { [GeneratedBindableCustomProperty([nameof(Value)], [])] sealed partial class Language3 : IProperties2