Initial proposal for P/Invokes via Source Generators

[AaronRobinsonMSFT]

Initial proposal for a new "Marshal Directive" concept. I am putting this into a design document instead of a issue. The enclosed API is a strawman that can be modified as we iterate.

/cc @jeffschwMSFT @jkotas @JeremyKuhne @jkoritzinsky @MichalStrehovsky @davidwrighton @stephentoub @vitek-karas @elinor-fung

[AaronRobinsonMSFT]

@lambdageek Feedback from the Mono side of things would also be welcome.

[jkotas]

Is this meant to be primarily used with source generators?

If this is primarily meant to be used with source generators, we should do all the work at build time - so that the runtime does not need to be looking up attributes and stitching things together.

If this is not meant to be meant to be used with source generators, I do not see the benefit. This basically allows you to write the PInvoke signature and the PInvoke marshaling code in two different .cs files instead of one. I believe that partial methods allow you to do that as well.

[AaronRobinsonMSFT]

If this is primarily meant to be used with source generators, we should do all the work at build time - so that the runtime does not need to be looking up attributes and stitching things together.

If I recall the Roslyn source generators proposal wouldn't allow modification of user defined code. I was starting from that premise. This attribute can be used by the source generator or user to provide a stub. If the new source generators proposal does allow user defined code to be changed, then I agree this proposal is much less interesting.

This basically allows you to write the PInvoke signature and the PInvoke marshaling code in two different .cs files instead of one. I believe that partial methods allow you to do that as well.

I don't see how methods adorned with a DllImportAttribute can be used with the partial keyword. Is the below even possible?

[DllImportAttribute("Kernel32.dll")]
extern static partial bool QueryPerformanceCounter(out long lpPerformanceCount);

[davidwrighton]

I believe that souce generators cannot modify user code, but this document doesn't really describe an interaction with source generators. I believe a more appropriate model here is to not have a DllImportAttribute at all, and instead rely on the source generator to provide an attribute syntax. As such, I don't think that the API should be part of the core library at all, but instead should be delivered as part of a nuget package that contains both the attribute definitions for how to define communication with the source generator, but I don't see any reason for there to be any logic added to the runtime at all to cover these scenarios.

[AaronRobinsonMSFT]

I believe that souce generators cannot modify user code

That is my expectation as well and some of the reason for a new attribute that would supplement the DllImportAttribute.

but this document doesn't really describe an interaction with source generators.

Was trying to avoid any mention because what I was proposing wouldn't need them. The source generators would merely be an optimization or automated way to generate the stubs on the users behalf.

but I don't see any reason for there to be any logic added to the runtime at all to cover these scenarios.

Yep. I chatted offline with @jkotas offline and he pointed me in what it sounds like the same direction you were thinking. I am going to update this document with that re-alignment. I will also be calling out source generators' limitations and workarounds as they relate to not editing user code.

[tannergooding]

What would be the benefit of this over using a tool like ClangSharp or CppAst to generate a blittable signature and provide simple wrappers? That is, what is the benefit over:

public unsafe bool QueryPerformanceCounter(out long lpPerformanceCount) {
    long result = 0;
    bool success = QueryPerformanceCounter(&result) != 0;
    lpPerformanceCount = result;
    return success;

    [DllImportAttribute("Kernel32.dll")]
    static extern int QueryPerformanceCounter(long* lpPerformanceCount);
}

https://sharplab.io/#v2:EYLgtghgzgLgpgJwD4AEAMACFBGAdAJQFcA7GASzDlwElTEB7ABwGVEA3MgYzigG4BYAFAoAzFgBMGAMIYA3kIyKsYklAgAzOBmD16AGwwBFQogCeABUTr6CSMW5T6JeAgAUTmBj31iAcy+MlgjWthD2cI7OAJRyCkrx3n4YCDyEep4AvBhoAoLx8Tr6GFCEnNxQUBhZxmZBIXYOTnRuAGQpJekxAIRZOXH5AXU2DRFNmcmp6bkDiigA7MWl5Xz9+avxANoAInp61GCMNjAAgjAwCGTAhPCuAEQA0ojEcHoi4rgAJru3UQC660ocAA2DBwAAeLmIGDIpCMJgQFiswzCjWciFciV8ACpBkjQuFIqQotMlABfISkoA

[AaronRobinsonMSFT]

@tannergooding The intended benefit is that the new attribute would provide this behavior without modification of existing code paths. This is largely a matter of degree, but adding a single attribute is much less in my opinion than altering any part of a function declaration. Starting from a new project there isn't much difference.

[AaronRobinsonMSFT]

@jkotas @davidwrighton @vitek-karas @tannergooding Please take another look. I think I have addressed your previous concerns.

[AaronRobinsonMSFT]

/cc @jaredpar @chsienki

[marek-safar]

@vargaz please review

[tannergooding]

I think this makes sense overall. It might also be interesting to see how an existing codebase (WinForms?) might utilize this, as it sounds like in the current source generator design, they'd need to do some refactoring (likely using the proposed analyzer) to make it work.

[AaronRobinsonMSFT]

It might also be interesting to see how an existing codebase (WinForms?) might utilize this, as it sounds like in the current source generator design, they'd need to do some refactoring (likely using the proposed analyzer) to make it work.

All great questions/suggestions. WinForms would be a nice test bed. I also what to understand how this impacts build times since the inner loop could be negatively impacted. Your observation about code change and how that goes is very interesting on a real code base.

[AaronRobinsonMSFT]

@agocke @jaredpar @chsienki Can you please take another look and ensure this plan doesn't contain any other assumptions that could cause issues with using Source Generators?

[AaronRobinsonMSFT]

@agocke @jaredpar @chsienki Ping. Not looking for a sign off just a sanity check on general plan. Thanks.

[agocke]

@AaronRobinsonMSFT I wouldn't rely on any new language features, including modifications to partial methods. That will require separate LDM approval.

[AaronRobinsonMSFT]

@agocke Thanks for the feedback. I think at this point the ability for this feature to use source generators and be successful it is dependent on that feature. I believe we can accept if the partial keyword work can't go through we will defer this plan until such a time as we can get consensus on that feature work. I will merge this since it does call out that requirement and if that changes we can update the proposal as necessary.

[agocke]

@AaronRobinsonMSFT Yup, I'm not saying that it's likely that the feature won't be approved in LDM, just that I can't guarantee it. Certainly I'm in favor of the feature.

[jkoritzinsky]

@agocke is there somewhere we need to open an issue to get it on LDM's schedule or backlog?

[agocke]

There's one here: dotnet/csharplang#3301

I'll mark it for triage