Addressing issues with generics on AOT

[manodasanW]

This PR addresses a couple of different AOT compat issues related to generics:

• Refactors the AOT source generator to not have the WinRTExposedType attribute take an array of interfaces but instead take a type which then returns the list of interfaces to put on the vftbl. This allows for the ability to do initialization of generic interfaces such as initializing the code generated version of the vftbl for the specific generic instantiation before returning the interface array.
• Updating WindowsRuntimeTypeAttribute to take the source generated guid signature for structs so that in the scenarios where we can't precompute the IID, we can still compute the IID without using reflection to iterate through all the struct fields.
• Making all the function pointer delegates blittable.
• Making authoring scenarios AOT and trimming compatible by having the generated authoring projection register a lookup table for the authoring metadata types which have the WinRTExposedType attribute for vftbl generation.
• Refactoring the generic collection interfaces and the generated generic interfaces to be AOT compatible. This is done through a couple of different ways. First, both the Do_ABI functions and the RCW functions of these generic interfaces are source generated. The source generated version mainly just handles the managed to ABI or the ABI to managed marshaling of the parameters and return and then calls a common function in the Methods static class for that interface to handle the actual implementation logic. These source generated versions are registered by calling the Init function in the new Methods static class which takes the ABI type as also a generic. These new static classes also have a fallback for projections which were not generated using an updated cswinrt version but the app they are used in happen to have an updated version of WinRT.Runtime.
• The source generated RCW functions are generated by the cswinrt tool itself based on the projections it is generating. To avoid generating the same ones multiple times, they are generated at the end and are shared across all the functions in that projection. This can mean that different projections have similar generated functions but the first one to register them will be the one that is used.
• The source generated Do_ABI vftable functions are generated by the source generator which analyzes the sources to determine the actual C# implemented types implementing WinRT interfaces that are passed to projected functions. Given there is a fixed number of generic interfaces, these implementation are mostly hand authored with format specifiers for the marshaler to use based on the generic type.
• Enabling all the functional tests to run for AOT.
• Added missing array marshal functions which should have been there before.
• Adds a new CsWinRTAotOptimizerEnabled property which is by default on. It is mainly there if after we end discovering some scenarios where our source generator behaves incorrectly. This will allow those folks to unblock themselves by disabling the optimizer especially if they aren't yet concerned about publishing for AOT or trimming.
• In of our previous perf changes, we removed the use of IDIC for our collection generic interfaces via the addition of the Methods static class and the Impl class. This is now mostly expanded for the remaining custom mapped interfaces and also the code generated ones like IAsyncOperation.

[Sergio0694]

Question: right now we have this WinRTExposedType, which links to a separate generated type with the details, implementing the interface, which is constructed at runtime using reflection. Could we not do the same as ComWrappers is doing instead, that is, make WinRTExposedType non-sealed, and then have the generator create a derived type directly implementing the interface, and use that to annotate the projection types? This way CsWinRT can simply look that up and cast directly to the interface type, no need to use Activator.CreateInstance and reflection to get an instance to invoke methods on. Basically it'd just generate this:

[FooWinRTExposedType]
partial class Foo
{
}

internal sealed class FooWinRTExposedType : WinRTExposedTypeAttribute
{
    public override ComWrappers.ComInterfaceEntry[] GetExposedInterfaces() => ...;
}

Then CsWinRT can just do type.GetCustomAttribute<WinRTExposedTypeAttribute>(true) and use that directly. Should also help startup a tiny bit since there's no more Activator.CreateInstance being called. And this also works downlevel just fine, as we don't even need any attribute generics.

Additionally, this means we don't even need the interface, we just override the abstract GetExposedInterfaces() method on the base WinRTExposedType, which is also slightly faster to invoke than an interface dispatch (same for casting to the attribute type rather than to the interface type). And also in general, it's just one less type we need.

What do you think? 🙂

[manodasanW]

This was one of my initial ideas during the refactor. This might still be preferrable to do, will need to do some comparisons. The main reason I switched to what I have was I was concerned about generating much more code for each struct, especially blittable ones, where we only generate the struct today and not any marshaling classes. With the current approach, I was able to use a generic type that I passed to share the implementation for structs. But after this PR is in, I am willing to try out the other approach again and see how much more it really adds to code size and if it is minimal. Also I was thinking this approach may make it easier in the future to move to generic attributes when we can, but with that said, not sure which is better perf.

[Sergio0694]

Leaving a few more notes in case it helps 🙂

Tried building this branch locally but I'm still getting about 490 warnings from WinRT.Runtime, most of them being incorrect or missing trim/AOT annotations (related to #1347). Is this expected, and would this just be fixed in a separate PR?

---

All this work is really great to see though, love CsWinRT getting some improvements! 🎉

[manodasanW]

Leaving a few more notes in case it helps 🙂

Tried building this branch locally but I'm still getting about 490 warnings from WinRT.Runtime, most of them being incorrect or missing trim/AOT annotations (related to #1347). Is this expected, and would this just be fixed in a separate PR?

All this work is really great to see though, love CsWinRT getting some improvements! 🎉

I plan on going through all the remaining warnings and looking at if they are in fallback code paths or are not needed anymore or wrong annotations. I wanted to first get the changes in that would address all known AOT and trimming compat issues.

[dongle-the-gadget]

Is it possible for this path to be-AOT compatible somehow? This change can cause some IIDs to be missing entirely, causing CreateCCWForObjectForABI to fail. One affected API is StorageFile.GetFileFromPathAsync.

Here's a screenshot of AsyncOperationCompletedHandler<StorageFile> in CoreCLR (left) versus NativeAOT (right) (provided by @hez2010):

[manodasanW]

This code path is meant to be the fallback. It looks like the first guid is the one that is missing with the other extra ones being duplicates. For this scenario, the WinRT AOT source generator should have generated the necessary vtable entries as part of the lookup table. Do you see any warnings for WinRTAotSourceGenerator failing to run? If not, are you able to check if there is a generated file called WinRTGlobalVtableLookup.g.cs and inside of it, do you see a reference to the mentioned type?

[hez2010]

Those missing IIDs are delegates, such as AsyncOperationCompletedHandler<T>.
It can cause any async operations to fail (not only StorageFile.GetFileFronPathAsync)

[hez2010]

I checked the WinRTGlobalVtableLookup.g.cs, it did exist but I didn't find the code generated for Windows.Foundation.AsyncOperationCompletedHandler<Windows.Storage.StorageFile>.

While there was one for Windows.Foundation.AsyncOperationCompletedHandler<bool>.

[manodasanW]

I added a test case to test this scenario and I discovered a different issue, but not this specific one you are running into. I think I know why you are seeing this. Today, you typically add a CsWinRT package reference to generate a projection for a set of WinMD files or for authoring scenarios. With these changes, you also need to add a CsWinRT package reference to make your library or app which consumes CsWinRT projected APIs AOT compatible. This is because we have a new source generator that we are using to generate some code in order to be AOT compatible specifically in the generic type scenarios. In the future we may look for over ways to have the source generator run without needing a CsWinRT package reference such as when using the net windows10 TFM.

Can you confirm whether your app or library where this AsyncOperationCompletedHandler is being added has a CsWinRT package reference. If not, can you add one and see if that helps. I assume you are testing this by manually building my changes, so you might not have a CsWinRT package built. If so, you might be able to directly add a reference to the source generator as we do in our functional tests here. Let me know if that addresses it.

[manodasanW]

Thanks for confirming. OpenStreamForReadAsync should be coming from here. I will need to enlighten the source generator to handle this scenario. I will also look into the other scenario you mentioned which it doesn't handle.

[hez2010]

I think you can generate code for async operations at the invocation instead of the await expression (an async call is not necessarily to be awaited directly):

i.e. adding the below code under if (invocationSymbol is IMethodSymbol methodSymbol && GeneratorHelper.IsWinRTType(methodSymbol.ContainingSymbol)) in GetVtableAttributesToAddOnLookupTable:

                    if (GeneratorHelper.IsWinRTType(methodSymbol.ReturnType))
                    {
                        var completedProperty = methodSymbol.ReturnType.GetMembers("Completed").FirstOrDefault() as IPropertySymbol;
                        if (completedProperty != null && completedProperty.Type.MetadataName.Contains("Async") && completedProperty.Type.MetadataName.Contains("`"))
                        {
                            vtableAttributes.Add(GetVtableAttributeToAdd(completedProperty.Type, GeneratorHelper.IsWinRTType, context.SemanticModel.Compilation.Assembly, false));
                        }
                    }

This can handle all async invocations and it works fine with the below code:

var file = await StorageFile.GetFileFromPathAsync(...);
using var stream = await file.OpenAsync(FileAccessMode.Read).AsTask().ConfigureAwait(continueOnCapturedContext: false);
using var sw = new StreamReader(stream.AsStreamForRead());
Console.WriteLine(sw.ReadToEnd());

[hez2010]

I created a PR for you to resolve the above issue: #1383

[Sergio0694]

Partially unrelated, but I'm very confused by this line in the generated code above:

IID = global::WinRT.GuidGenerator.GetIID(typeof(global::Windows.Foundation.AsyncOperationCompletedHandler<global::Windows.Storage.StorageFile>).GetHelperType())

Given this is all generated code, can't the generator precompute that IID into some static field as usual (like all new AOT-friendly projections are also doing in this PR) and just use that here, rather than using GuidGenerator?

[manodasanW]

I left this for now as I have plans to try to remove the delegates from the lookup table if possible and move it into the projection itself similar to the rcw instantiations.

[Sergio0694]

Doesn't have to be fixed in this PR, but just for additional info — I've tried to use the CsWinRT package from this PR (as of 2a7b754) and can confirm that #1369 still happens (same repro project), same exception and stack trace.

[hez2010]

Hit another exception "Write lock may not be acquired with read lock held."
At WinRT.ComWrappersSupport.RegisterTypeComInterfaceEntriesLookup.
This exception will happen when you try to call WASDK. For example, in a WinUI 3 app.

[manodasanW]

Hit another exception "Write lock may not be acquired with read lock held." At WinRT.ComWrappersSupport.RegisterTypeComInterfaceEntriesLookup. This exception will happen when you try to call WASDK. For example, in a WinUI 3 app.

I did also run into this during my validation, working on a fix.

[jlaanstra]

I did something similar in InterfaceIIDs. Do we want to remove those?

[manodasanW]

Possibly, will handle cleanup in separate PR.