Roslyn: C# Proposal: Allow `extern` local functions

[agocke]

Motivation

For PInvoke methods you often want to provide an extern signature for the externally available method, but then immediately provide a "managed" wrapper around that signature for use in the rest of your program. However, because even private members can be accessed by other members in the same class there's no way to completely scope away a PInvoke signature to only a single wrapping method.

In addition, some PInvoke members have specific requirements for their use that must be done in the wrapper code e.g., acquiring a lock before usage.

Proposal

Allowing the extern modifier for local functions would provide functionality for both cases.

For example:

internal static int DefaultModelCompareAssemblyIdentity(
    string identity1,
    bool isUnified1,
    string identity2,
    bool isUnified2,
    out bool areEquivalent,
    out AssemblyComparisonResult result,
    IntPtr asmConfigCookie)
{
    lock (s_assemblyIdentityGate)
    {
        return CompareAssemblyIdentityWithConfig(
            identity1,
            isUnified1,
            identity2,
            isUnified2,
            asmConfigCookie,
            out areEquivalent,
            out result);
    }
}

[DllImport("clr", CallingConvention = CallingConvention.StdCall, CharSet = CharSet.Unicode, PreserveSig = true)]
private static extern int CompareAssemblyIdentityWithConfig(
    [MarshalAs(UnmanagedType.LPWStr)] string identity1,
    bool isUnified1,
    [MarshalAs(UnmanagedType.LPWStr)] string identity2,
    bool isUnified2,
    IntPtr asmConfigCookie,
    out bool areEquivalent,
    out AssemblyComparisonResult result);

would become

internal static int DefaultModelCompareAssemblyIdentity(
    string identity1,
    bool isUnified1,
    string identity2,
    bool isUnified2,
    out bool areEquivalent,
    out AssemblyComparisonResult result,
    IntPtr asmConfigCookie)
{
    [DllImport("clr", CallingConvention = CallingConvention.StdCall, CharSet = CharSet.Unicode, PreserveSig = true)]
    extern int CompareAssemblyIdentityWithConfig(
        [MarshalAs(UnmanagedType.LPWStr)] string identity1,
        bool isUnified1,
        [MarshalAs(UnmanagedType.LPWStr)] string identity2,
        bool isUnified2,
        IntPtr asmConfigCookie,
        out bool areEquivalent,
        out AssemblyComparisonResult result);

    lock (s_assemblyIdentityGate)
    {
        return CompareAssemblyIdentityWithConfig(
            identity1,
            isUnified1,
            identity2,
            isUnified2,
            asmConfigCookie,
            out areEquivalent,
            out result);
    }
}

Known Issues

The main problem with doing this is that to make a proper PInvoke signature you also need to provide attribute support for both the member and the parameters, which is also not supported at the moment.

[HaloFour]

Is it really that necessary to scope said P/Invoke methods to only their wrapping method? The method would be private to the declaring type either way. This would only buy you the unutterable name, which doesn't change much since the method couldn't be called externally to the declaring type anyway.

Typically if I'm wrapping P/Invoke calls it's because I need or want to transform the arguments into anew easier to use form. Given the lack of local types you'd still be stuck defininginx those types nested within the declaring type, at best. Then again, if we did have local types _and_ local extern functions then you'd have the whole mess stuffed into the method ... just because?

[GSPP]

This is really nice. Often, a PInvoke method is wrapped to provide a more convenient API to callers. For example, the typical if (!f()) throw new Win32Exception(); check.

Making the PInvoke function a local function improves code quality by bundling the two inseparable methods together into one artifact. This should improve legibility and tooling.

[svick]

@HaloFour That sounds like argument against local functions in general, since it's not really specific to extern local functions.

[alrz]

I do like the locality of the declaration and the usage. A static class like NavtieMethods is useful only when you are considering reusability, other than that, an extern local function would be a better approach in terms of coherency. I think compiler intrinsics (#11475) would be another compelling use case for this feature, especially those that can be defined with different signatures e.g. LoadMethodToken so you can only define the required overload and there would be no need to cast your method group to select a specific one.

[agocke]

@HaloFour I agree it's not a game changer, just a quality of life issue. IMO, it makes constructs like the example I posted easier to understand. There's no possibility anyone would accidentally call the PInvoke method without locking with the rewrite.

[HaloFour]

@svick

That sounds like argument against local functions in general, since it's not really specific to extern local functions.

That might be true if local functions didn't have lexical scoping and close over the scope of their parent method. That feature is reason enough for local functions, IMO, and it's something that extern local functions can't take advantage of. So the only advantage of an extern local function is method-private scoping. I'm not saying that it's not an advantage, but I can't imagine that it comes up often.

@agocke

I agree it's not a game changer, just a quality of life issue. IMO, it makes constructs like the example I posted easier to understand.

Does it really improve the readability? It just shuffles things around. Sure, you might be able to discern a little more obviously that the P/Invoke call is used by only that one method, and it might help prevent you from accidentally calling it directly from some other method in that class, but I can't imagine that those are terrible common scenarios. And what about any of the dependent structs required to call that external function? Those would still have to be declared somewhere else, so you're not fully bundling everything into one self-contained method anyway.

Shrug, I don't particularly care, I just don't see this being much of an improvement of how I write P/Invoke calls today, and I'm not of the opinion that everything needs to be or should be jammed into one method body. 😀

[SLaks]

This currently gives a very subpar error experience: #16098

[jnm2]

I really like this proposal. I can see myself using it.

I'm a little conflicted because I'm used to following CA1060. What's the official guidance going to be with respect to this?

[jcouv]

This was implemented along with attributes on local functions (as part of C# 9). Closing the issue.