Runtime: Exception to throw when interface dispatch is ambiguous

[MichalStrehovsky]

Proposal

The default implementation of interface methods feature requires an exception to be thrown for the case of a diamond inheritance situation.

The exception signifies that there's no most specific implementation of an interface method provided by a class.

The situation can occur in a case like this:

In the above situation, the dispatch to IFoo.Frob() in Main is ambiguous between IBar.Frob and IBaz.Frob.

We need an exception type that would be suitable for the runtime to throw in this situation.

namespace System.Runtime
{
    [Serializable]
    public sealed class AmbiguousImplementationException : Exception
    {
        public AmbiguousImplementationException();
        public AmbiguousImplementationException(string message) : base(message);
        public AmbiguousImplementationException(string message, Exception inner);
        protected AmbiguousImplementationException(SerializationInfo info, StreamingContext context);
    }
}

Alternatives considered

System.Reflection.AmbiguousMatchException. The problem is that this is not a reflection operation and throwing an exception from a Reflection namespace might misleadingly hint that reflection is involved.

[danmosemsft]

Aside: this exception like all our exceptions should be implemented with [Serializable]

[bendono]

Is this case supposed to compile? Ideally I would rather get a compilation error than a runtime error. That said, I could imagine implementations written directly in IL, in which case an exception would be needed. I like the System.Runtime proposal.

[MichalStrehovsky]

Is this case supposed to compile?

The C# compiler is going to error out on this, so it won't compile.

But one can still end up in this situation through e.g. NuGet packages: Let's say there are two NuGet packages: A and B. B depends on A. When B was compiled, everything was fine. In a new version, A adds a default interface implementation that introduces the diamond pattern in B. B is not recompiled since. App uses latest versions of both A and B. App compiles fine because the diamond pattern is within B. At runtime, we hit this situation while executing the code of B.

Mind you, adding default interface implementation like this will be considered a breaking change, so technically, so A is doing something wrong (providing a default implementation of an existing interface on an existing interface). But the default interface methods feature has the unfortunate property of making this non-obvious.

[davidwrighton]

Could you specify what the message would be at least in our initial implementation? I don't think it needs to be formalized, but an idea of what the exception text would look like would be helpful.

[MichalStrehovsky]

Could you specify what the message would be at least in our initial implementation?

Updated the proposal with the existing exception message we use in CoreCLR: "Could not call method '%1' on interface '%2' with type '%3' from assembly '%4' because there are multiple incompatible interface methods overriding this method."

[terrajobst]

Video

Looks good. A few things:

• We should have a parameterless constructor
• We should add the serialization contructor
• We should add the inner exception constructor
• We should mark the exception serializable
• We should seal the type

[MichalStrehovsky]

Marking easy/up for grabs if someone from the community would like to add a brand new exception type.

This would be work in the CoreCLR repo to add it to System.Private.CoreLib and once the change propagates, add it to the System.Runtime contract in this repo (@terrajobst can you confirm the contract name please?)

[SwooshyCueb]

@MichalStrehovsky Submitted a PR to the CoreCLR repo. (dotnet/coreclr#22280)

[terrajobst]

I've edited the issue description so that the API shape matches our feedback.

[terrajobst]

@danmosemsft

Can we assign a dev to expose this API? This way, it can also be exposed from .NET Standard.

[danmosemsft]

Sure. @maryamariyan since you did the last one, would you mind doing this one? Hopefully it is very similar - you can start by just copy/paste implementation and tests.

[harish-kumar13]

was the new error type created in CoreLib and Used to throw? or is this still up for grabs by the community?

[MichalStrehovsky]

was the new error type created in CoreLib and Used to throw? or is this still up for grabs by the community?

Yes, it was done in the pull requests referenced above. What's left is to expose it from a contract in this repo and to add a test.