[CodeGen] Always specify grain extension interface for grain extension calls

[ReubenBond]

In #8749, we changed how we generate invokers (essentially, closure classes which represent grain method invocations) so that we only generate one invoker for each original method definition. That was in response to an issue uncovered by one of the analyzers which @ledjon-behluli added: https://discord.com/channels/333727978460676096/922948121619751012/1179628138490232923, where it was previously impossible to add aliases to some types.

Unfortunately, that caused an issue, which @oising hit: for grain extensions, we need to know which extension interface to fetch from the grain so that we can execute the right method against it.

The following code snippet corresponds to the grain extension test interface added in this PR.
Notice that the target field is typed based on the original method definition's interface, while SetTarget uses the grain extension's interface. That is the desired outcome, ensuring we get the right extension while also calling the correct method overload.

[global::System.CodeDom.Compiler.GeneratedCodeAttribute("OrleansCodeGen", "8.0.0.0"), global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)]
[global::Orleans.CompoundTypeAliasAttribute("inv", typeof(global::Orleans.Runtime.GrainReference), "Ext", typeof(global::UnitTests.General.IMyGrainExtension), "9DED4F7A")]
internal sealed class Invokable_IMyGrainExtension_GrainReference_Ext_9DED4F7A : global::Orleans.Runtime.Request
{
    public int arg0;
    global::UnitTests.General.IMyRegularInterface target;
    private static readonly global::System.Reflection.MethodInfo MethodBackingField = OrleansGeneratedCodeHelper.GetMethodInfoOrDefault(typeof(global::UnitTests.General.IMyRegularInterface), "SetExtensionValue", null, new[] { typeof(int) });
    public override int GetArgumentCount() => 1;
    public override string GetMethodName() => "SetExtensionValue";
    public override string GetInterfaceName() => "UnitTests.General.IMyRegularInterface";
    public override string GetActivityName() => "IMyRegularInterface/SetExtensionValue";
    public override global::System.Type GetInterfaceType() => typeof(global::UnitTests.General.IMyRegularInterface);
    public override global::System.Reflection.MethodInfo GetMethod() => MethodBackingField;
    public override void SetTarget(global::Orleans.Serialization.Invocation.ITargetHolder holder) => target = holder.GetComponent<global::UnitTests.General.IMyGrainExtension>();
    public override object GetTarget() => target;
    public override void Dispose()
    {
        arg0 = default;
        target = default;
    }

    public override object GetArgument(int index)
    {
        switch (index)
        {
            case 0:
                return arg0;
            default:
                return OrleansGeneratedCodeHelper.InvokableThrowArgumentOutOfRange(index, 0);
        }
    }

    public override void SetArgument(int index, object value)
    {
        switch (index)
        {
            case 0:
                arg0 = (int)value;
                return;
            default:
                OrleansGeneratedCodeHelper.InvokableThrowArgumentOutOfRange(index, 0);
                return;
        }
    }

    protected override global::System.Threading.Tasks.ValueTask InvokeInner() => target.SetExtensionValue(arg0);
}

Microsoft Reviewers: Open in CodeFlow

[ReubenBond]

I added another commit: we should always use explicit interface implementations when generating proxies. this lets us correctly handle ambiguities we couldn't correctly handle before. The code is also simpler.

[alrz]

internal sealed class Invokable_IMyGrainExtension_GrainReference_Ext_9DED4F7A : global::Orleans.Runtime.Request

Is the internal access also part of this PR? I get a public type derived from TaskRequest..

That would resolve the issue with internal payload classes (and internal interface methods).

[ReubenBond]

@alrz currently, we only consider the visibility of the interface itself, but we could take a change to also consider the visibility of the method, if that's valid, and generate the invoker using the least visible of the two. See InvokableGenerator.cs:49-55

[alrz]

@ReubenBond

That would definitely work but I'm wondering if there's any reason at all to make it anything other than internal (or even file if it makes sense, to make the name conflict-free), if I'm not mistaken, orleans is the only consumer of these types anyways.

[ReubenBond]

@alrz
The reason is that when you have an interface in one assembly which references an interface in the other, both generated GrainReference classes need to reference the common generated invoker by name. If it's file-scoped or less accessible than the interface, that would not be legal C# and we need to emit legal C# because we cannot suppress accessibility rules for generated code.

[alrz]

The reason is that when you have an interface in one assembly which references an interface in the other, both generated GrainReference classes need to reference the common generated invoker by name.

In that case, wouldn't we generate the whole thing inside the "current" project including the types related to the external interface? that is, the client wouldn't have any knowledge if the referenced interface from another assembly is indeed a grain. If that's correct I think file-scoped types should work just fine?

[alrz]

Sorry I think you're saying the interface AND the grain impl are sourced from outside, in which case we're going to lookup invokers from the referenced assembly. I don't know where this happens or whether or not we use literal C# to do that but afaik Activator is able to create types no matter where they are declared.

[ReubenBond]

The Grain impl isn't an issue here. The scenario I'm talking about is when you have:

// in Asm A
interface IMyDerived : IMyBase
{
  ... methods ...
}

// in Asm B
interface IMyBase : IGrain
{
  ... methods ...
}

We need to generate IInvokable implementations for the methods defined in IMyBase and for the methods defined in IMyDerived. We also need GrainReference implementations which implement those interfaces and create instances of those IInvokable implementations for the corresponding methods.

The way it works now, we only generate the IInvokables for IMyBase once, in Asm A. The GrainReference in Asm B needs to be able to reference those IInvokables by name. If Asm B cannot see A's internals, we would not be able to faithfully generate the invokables. In that case, I think we should error out.