Proposal: Customisable overload resolution

[popcatalin81]

Customisable overload resolution

This is a proposal to add a customization mechanism for the C# language, to control method group overload resolution.

Summary:

C# overload resolution is designed for best possible backward compatibility in the general case. Some special types of libraries and evolving APIs meet some difficulties in implementation when their needs diverge from the C# language rules. IE: instead of being focused on compatibility they are focused on performance or improved functionality :

Motivation:

• API evolution: Allow library writers more control over overload resolution in API evolving scenarios like migration from non-generic to generic, or from string to FormattableString APIs.
• Performance work: Allow library writers to chose nonboxing conversion overloads over boxing overloads.

Design

• Define a new a new [PreferredOverloadAttribute] to designate higher priority method overloads from a valid method group.
• The attribute will have a PriorityBoost integer property to allow specifying different priorities for valid candidate methods.
• For methods with the same PriorityBoost value, the regular overload resolution rules will apply.

Formattable string example:

class Program {
	static void Main()
	{
		Write($"Hello the date is  {DateTime.Now}");	
	}
	static void Write(string text)
	{
		Console.WriteLine("unformated");
	}

        [PreferredOverload]
	static void Write(IFormattable text)
	{
		Console.WriteLine("formatted");
	}

        [PreferredOverload(PriorityBoost = 2)]
	static void Write(FormattableString text)
	{
		Console.WriteLine("formattable string");
	}
}

This is a case where a structured Logging library will indicate the preference to use the formattable string overload over the string overload in order to enhance its API usage.

C# 7.2:
unformatted

C# 7.2+:
formattable string

ISpanFormattable example:

class Program {
	static void Main()
	{
		Write(1);
	}
	
	static void Write(object value)
	{
		Console.WriteLine("object");
	}

	[PreferredOverload]
	static void WriteT<T>(T value)
			where T:ISpanFormattable
	{
		Console.WriteLine("ISpanFormattable");
	}
}

This will allow libraries to take advantage of newer performance oriented features without introducing new method names that will require source changes, only recompilation with the latest compiler:

C# 7.2:
object

C# 7.2+:
ISpanFormattable

params Span<T> example

Champion "params Span<T>"

[PreferredOverload]
void Foo(params Span<int> values);

void Foo(params int[] values);

This will allow method overload resolution to chose the Span<T> based overload instead of the int[] overload chossing the allocation free path,

[jnm2]

Same (in principle) as #964?

[bondsbw]

This could lead to subtle bugs where the consumer of your API would get different overloads depending on which version of C# is in use.

[jnm2]

@bondsbw Not if these only came into play where there would previously have been a compile error due to overload ambiguity.

[alrz]

C# overload resolution is designed for best possible backward compatibility in the general case.

I don't think that's the case, overload resolution is designed to be more predictable and less surprising, when it comes to backward compat, you're almost alway on your own.

but I agree, OverloadResolution.cs is not long enough to be actually useful.

[bondsbw]

@jnm2 Good point, though the examples in the proposal already have valid resolutions.

[popcatalin81]

This could lead to subtle bugs where the consumer of your API would get different overloads depending on which version of C# is in use.

I can see two scenarios for this issue:
A) This is acceptable. There's a small risk, only specific targeted libraries will use this, and authors will ensure compatibility.

B) This should be prevented and only newer C# compilers allowed to compile such code.
Perhaps there could be some mechanism involved where older compilers will raise a compilation error ? New syntax seems a little too much for this feature, would there be a way to make the attribute accessible to only a certain version of the compiler?

I don't think that's the case, overload resolution is designed to be more predictable and less surprising, when it comes to backward compat, you're almost alway on your own.

Overload resolution has been "surprising" to me (and others I've tutored over the years) since the introduction of generics continuing to string interpolation.

I think foo(string s) vs foo(FormattableString s) called with foo($"{bar}) is pretty surprising, and so is generic vs non generic method overload resolution lots of times.

[Joe4evr]

This could lead to subtle bugs where the consumer of your API would get different overloads depending on which version of C# is in use.

There have already been cases where a major version of C# changed overload resolution in places that the resolution failed to do the thing API authors expected (aka "Betterness").

C# overload resolution is designed for best possible backward compatibility in the general case.

If this was true, then Betterness/Better Betterness would never have happened and Bestest Betterness wouldn't even be on the table.

[Neme12]

@Joe4evr But doesn't "betterness" only affect code that didn't previously compile?

[jnm2]

@Joe4evr The second sentence from your link to Bestest Betterness:

This has to be done carefully to preserve backward compatibility.

I think @Neme12 is right.

[Neme12]

Also:

Per LDM today, since it not technically a change to overload resolution, but rather the steps before (weeding out candidates) and after (removing any redundant logic to final verification). Should we rename the issue?

This would not only make breaking changes to overload resolution itself (not just removing some candidates), but also make it customizable? That seems like a disaster. Overload resolution is already complicated as it is. But at the very least you can expect it to be the same for everyone.

[Neme12]

Is it really that hard to just make a new method name?

[VisualMelon]

@Neme12 the OP discuss concerns with updating existing APIs, implying that the old (less ideal) method will still exist, and duly the new (less terrible) method will be competing for the name, so this is kind of rejected by assumption for the use-cases.

[popcatalin81]

This has to be done carefully to preserve backward compatibility.

I don't think this feature is about backward compatibility, I don't want backward compatibility here, I want to explicitly change overload resolution priority when targeting a newer language version.

I specifically want improved functionality by using a newer compiler. (IE: choosing the FormatedString overload opposed to the string one).

Is it really that hard to just make a new method name?

To give a more specific example:

I'm developing a template based code generator based on C# 6.0 interpolated strings (If you want to see some details about it you can see here but for the purpose of the discussion I'll replicate the examples here). In the generator, FormatedString is used to enhance the features of the generator, some examples:

Write($" This is a status message: {(Success, ConsoleColor.Green)}"); // this adds colorisation to output
Write("A different Message"); // /Can't have this overload ... as this is a string one, and the previous one won't work
// so instead need a new name
WriteS("A different message"); // This works, different method name, but now you can do this:
WriteS($" This is a new status message: {(Success, ConsoleColor.Green)}"); // which doesn't work because this is a string overload 
//and now you have to juggle between overloads

Same with logging, I would like to contextually choose to log structured and unstructured messages based on whether I use a string or a formatted string, not based on method names.

[Neme12]

@VisualMelon What's wrong with making a new name for the "less terrible" method? And as I understand it, it's not that the old method is "terrible" anyway. You don't always need interpolation. When you do, you call a different method that has a "Formattable" or some other suffix. And when you use that name, at least you can be sure that this is going to do the right thing.

Let's say if we take the example of SQL and auto-converting interpolation to SQL parameters. In this case, you really want to be explicit about this and not rely on overload resolution (especially when it differs between different language versions!) to prevent SQL injection. With a different method name, this will also be a lot less fragile - if you accidentally pass in a string, the code won't compile.

[Neme12]

@popcatalin81 Why can't you make a new method called WriteFormatted that only takes a FormattableString? Why do you need to change the behavior of people's existing code? That doesn't sound like a good idea. If they want colorization, they can opt in with the new method. What if they happened to use interpolation for something completely unrelated to what they want colorized?

[Neme12]

Also, people using (Success, ConsoleColor.Green) in an older C# version would be out of luck for no apparent reason.

[popcatalin81]

@Neme12, the only reasons for this proposal are niceness of the API (IE: not using multiple method names) and to avoid juggling with the names in the code (The entire reason method overloads exist is for niceness, otherwise different names can be used where overloads are currently allowed)

[Neme12]

I'm sorry but if the overloads have a different behavior, then they should have different names. We're really going overboard with this here. There are languages that don't have method overloading at all and you can still get around just fine.

The entire reason method overloads exist is for niceness

You can make more than 1 nice name. In fact a descriptive name might be much nicer than 1 that does different things depending on what you pass in, and then having to wonder what it will actually do.

I don't find WriteFormatted to be an ugly name, but I guess we're going to disagree here.

[jnm2]

Slightly different, but I really regretted writing a couple dozen overloads named Write even though they all took different domain types because it's so much harder to Alt+\ to the one I want.

[Neme12]

Note: I'm fine with overloading and find it OK to have overloads for different data types as long as you can think of them as having the same behavior, eg: Write(int), Write(string), but it would not be OK to have Write(int) write a colored text.

In the same way, I could imagine having Write(string) and having both WriteFormatted(FormattableString) and WriteFormatted(string, IEnumerable<(TextSpan, Color)> colorizedSpans) - here WriteFormatted has 2 overloads but I'd be personally OK with this because they have the same purpose. Write, on the other hand, is just plain old non-formatted output. So the different name isn't to clarify the data type used - it's really to clarify what you want the result to look like.

[HaloFour]

IIRC the crux of the issue here isn't really overload resolution, it was an explicit design decision around string interpolation. The interpolation expression itself has no type. It has three possible implicit conversions, to string, FormattableString and IFormattable, in that order. So when you're trying to pass such an expression to a method the compiler will look for suitable implicit conversions and will resolve the type to String before it does FormattableString and find those overload candidates as a result. That might fall under the overload umbrella when considering implicit conversions. IIRC this was special-cased with interpolated strings so that they wouldn't cause existing code to break due to ambiguous overloads but I don't recall the entire debate and the CodePlex archives aren't the most fun to trawl through.

[jnm2]

@HaloFour I think @popcatalin81 is aware.

[popcatalin81]

@jnm2 yes, fixing the overload resolution for everyone would be a breaking change. However, making it as an option, only for some opt-in libraries, might be doable. (Especially logging libraries would opt-in to this)

[Neme12]

fixing the overload resolution

Fixing implies it's currently broken. I don't think so. Consider this piece of code:

var str = $"hello {x}";

Would you want str to be of type FormattableString as opposed to string here? I personally would not. Interpolated strings are primarily designed to be interchangable with regular string or string.Format.

[popcatalin81]

@Neme12 Yes!

I would strongly want str to be FormattableString, but I probably have a different set of use cases in mind.

Also I think FormattableString and string can be easily made interchangeable by adding an implicit conversion from FormattableString to string. Although this might bring a small overhead in some circumstances (the extra object and possibly multiple conversions to string) I think it's worth it.

Example:

var str = $"hello {x}";
DisplayLocalized(str);

I would prefer str to be FormattableString here, as it opens up possibilities to nicer APIs like encapsulating Culture related responsibilities in other components.

Other examples, include structured logging and live templating.

[ufcpp]

I recall similar discussion:

Task and ValueTask overload resolution
dotnet/roslyn#7169 (comment)

Proposal: language support for Obsolete
dotnet/roslyn#11583

[Neme12]

I would prefer str to be FormattableString here, as it opens up possibilities to nicer APIs like encapsulating Culture related responsibilities in other components.

If you do, you can always specify the type yourself:

FormattableString str = $"hello {x}";

But understand that this is not the majority scenario. Most of the time, people just want a regular string.

[popcatalin81]

If you do, you can always specify the type yourself:

Yes you ca do this:

FormattableString str = $"hello {x}";`

but if you inline your variable you also need to do this (if you have both string and formattable string overloads):

DisplayLocalized((FormattableString)$"hello {x}");

[popcatalin81]

Another (I think) great use case has popped up: params Span<T>

The following would make method resolution chose the params overload over the int array overload:

[PreferredOverload]
void Foo(params Span<int> values);

void Foo(params int[] values);

Edit: fixed sample

[popcatalin81]

@HaloFour I think it should

If you want your library to be both backward compatible and also have the Span<T> overloads then the compiler should allow it.

[HaloFour]

@popcatalin81

Removal of the params keyword from a method doesn't change the binary signature and wouldn't break any existing consumer. It doesn't make sense to have multiple params methods.

[popcatalin81]

@HaloFour it changes the IL signature because it removes the [ParamsAttribute] from the method parameter https://msdn.microsoft.com/en-us/library/system.paramarrayattribute(v=vs.110).aspx

This will break backward compatibility by making older compilers give compilation errors.

[gafter]

I assume that the C# compiler won't allow params on two methods that would cause such an ambiguity?

The C# compiler won't prevent doing something that the language specification permits.

[HaloFour]

@popcatalin81

Potentially breaking source compatibility is expected when you modify and add signatures to an object. It can always effect overload resolution.

@gafter

The C# compiler won't prevent doing something that the language specification permits.

The language specification doesn't allow using params on anything except arrays currently, so clearly we aren't going by what the specification current says. If the compiler would allow adorning two methods with params where the only difference is int[] vs IEnumerable<int> vs. Span<int> or whatever then the specification would also clearly need to state which would be considered the best overload.

[bondsbw]

@popcatalin81

This will break backward compatibility by making older compilers give compilation errors.

Would the new params overload somehow be usable by the old compiler? If so, it would still compile, just using the new overload.

[HaloFour]

@gafter

Just to clarify, is it your opinion that after params Span<T> and/or params IEnumerable<T> are/is implemented that the following would be legal C#?

using System;
using System.Collections.Generic;
public class C {
    public void M(params int[] a) { }
    public void M(params Span<int> a) { }
    public void M(params IEnumerable<int> a) { }
}

If so, which overload would be invoked for M(1, 2, 3)? Would it be possible to explicitly disambiguate which overload you wanted without explicitly passing an array, span or enumerable?

I just want to make sure that we're on the same page.

[gafter]

@HaloFour It is my opinion that if we extend params to both IEnumerable<T> and Span<T>, that those declarations would be legal. There would be an error at the call site because there are two candidates neither of which is "better" than the other. The int[] overload would be better than the IEnumerable<T> overload so the IEnumerable<T> overload is out of the running.

The proposals to extend params do not include any mechanism for controlling overload resolution, and I do not think a proposal to influence overload resolution would be likely to be accepted.

[HaloFour]

@gafter

There would be an error at the call site because there are two candidates neither of which is "better" than the other.

Sounds like there'd be no value in applying params to multiple overloads like that, then. At that point why not just make it illegal to define those overloads at all?

[jnm2]

@HaloFour Compatibility with older compilers/language versions?

[HaloFour]

@jnm2 By being incompatible with newer compilers?

[jnm2]

@HaloFour I don't understand why it would be incompatible with newer compilers. I'd want a params int[] in case the langversion or compiler version is lower, and a params Span<int> which newer compilers would always pick instead.

[HaloFour]

@jnm2

and a params Span<int> which newer compilers would always pick instead.

No, newer compilers would see both params int[] and params Span<int> and fail to compile due to the ambiguity at the call site, based on @gafter's comment above.

[jnm2]

@HaloFour Wouldn't have to be the case. We could change the rules and make it smarter.

[gafter]

At that point why not just make it illegal to define those overloads at all?

@HaloFour We don't need a reason not to do things. What precise rule are you proposing that would forbid this set of method declarations?

We could change the rules and make it smarter.

@jnm2 What precise rule changes are you proposing that would make it "smarter"? Overload resolution is one of the most delicate areas of the language and we are reluctant to touch it without very good cause and proof that the change is sound. I'm not confident that we even have cause for a change here.

[jnm2]

@gafter The rule would be that if all the overload candidates are params, pick the Span<> one if it exists. What's your plan for people who need to make sure their libraries don't introduce breaking changes for older compilers which are compiling against an existing params int[] method? Just not to use params on their new Span<> overload?

[HaloFour]

@gafter

We don't need a reason not to do things. What precise rule are you proposing that would forbid this set of method declarations?

I don't disagree. I just think that it's a subpar experience to be able to have this combination of overloads marked with params when the compiler will not actually let you invoke them with variable arguments. That could be the result of a mistake on the part of the developer, or perhaps the intention that @jnm2 has where they want to expose a new and better params Span<T> overload while retaining downlevel compiler support for params T[]. If the combination of the overloads makes the feature unusable then I would think that the compiler would prevent the developer from declaring that combination of overloads.

That said, I'm probably in agreement with @jnm2 that given the option of params T[] and params Span<T> that the compiler prefer the latter.

[gafter]

The rule would be that if all the overload candidates are params, pick the Span<> one if it exists.

Even if other parameter positions make some other method preferable for other reasons (e.g. parameter type of int vs object)?

What's your plan for people who need to make sure their libraries don't introduce breaking changes for older compilers...

We don't even have a "plan" to add support for params Span<T> yet, so it would be premature to describe all the details of such a plan.

[jnm2]

@gafter No, only if all other things are equal. The preference for Span<> would be the least significant preference.

We don't even have a "plan" to add support for params Span<T> yet, so it would be premature to describe all the details of such a plan.

I'm answering your question to me with a question which it seems you'll inevitably have to deal with if the championed proposal #1757 is to go forward.

[gafter]

I'm answering your question to me with a question it seems you'll inevitably have to deal with...

I don't agree that your question is one we'll have to deal with.

[jnm2]

@gafter I'm sensing some kind of pushback to our questions. I apologize if I've said something out of place.
Any action you take on #1757 will be making a choice one way or the other: do you want it to be possible for users to add params Span<T> overloads without having to remove params from the existing method, causing source-breaking changes in lower versions? Even if "yes" would fall out by default (and I hope it's "yes") it's still a question that has been raised in which we're interested.

[tannergooding]

What's your plan for people who need to make sure their libraries don't introduce breaking changes for older compilers...

We don't even have a "plan" to add support for params Span yet, so it would be premature to describe all the details of such a plan.

I'm answering your question to me with a question which it seems you'll inevitably have to deal with

I don't agree that your question is one we'll have to deal with.

I think this is a question that almost every language feature has to deal with. That is, for any given language feature, you basically need to ask (IMO): "Can CoreFX use this?" (or replace CoreFX with other library with strict back-compat, like Roslyn, etc). And also ask:

• Can it be used with existing types?
• Can it be used with existing APIs?
• Can it be used with a new overload for an existing API?
• New APIs and New types are generally "ok", but this should be asked as well

I think, for the most part, the language has been fairly good in this regards (see readonly struct), but in other areas it has been lacking and can make life difficult (see in parameters, or ref returning properties). I also think that params Span<T> and/or params IEnumerable<T>, if they were implemented, would need to answer this question. If it isn't asked and answered, then the answer to "Can CoreFX use this?" could end up as "not as a new overload for an existing API", which greatly lessens the value of the feature (IMO).

That being said, given that params is not part of the IL method signature (it is just attribute data attached to the parameter), it should be safe to accomplish this by changing Method(params T[] x) to be just Method(T[] x) and then just adding a new Method(params Span<T> x) overload. This would maintain binary compatibility (since existing code could continue resolving Method(T[]), which is the IL signature already emitted) and new code would compile to use Method(params Span<T>)

[HaloFour]

@tannergooding

I agree. But I also think that @jnm2 has a really good point. If the BCL were to start adding new params Span<T> overloads and, as you say, removing params from the existing T[] overloads, that would break all existing projects that are using downlevel compilers. Because of that I suspect that CoreFX would never consider migrating to using params Span<T> for existing APIs.

The solution is somewhat easy. Support params on both and have the compiler prefer Span<T> over T[] as a final elimination of overload candidates.

[tannergooding]

@HaloFour, right. CoreFX has some additional considerations since they can/will be used from multiple compiler versions. In that case, they may need to keep both params T[] and params Span<T>, and the compiler would have to provide some default in order for CoreFX to use the feature.

in parameters are another example of this (but somewhat in the opposite direction). Older compilers (for non-virtual methods) see it as a regular ref and let you call it using ref. But, once you upgrade to a compiler that understands ref readonly, the compiler sees it as an in and passing a parameter using ref for them is illegal.

[gafter]

@jnm2 I'm not intending to push back against the asking of questions, I'm just not convinced in this case that an answer is needed. I'm very reluctant to modify overload resolution in general because it is one of the most delicate areas of the language.

I believe that API authors can reasonably migrate as @tannergooding has suggested. This may require that program authors are capable of specifying which version of a platform API they are compiling against. If you want to use new versions of the platform APIs, you might have to upgrade both your platform version and your compiler. Of course we need to make sure that VB.Net, F#, and any other widely used languages have new versions that understand the new constructs.

[HaloFour]

@gafter

I'm very reluctant to modify overload resolution in general because it is one of the most delicate areas of the language.

I understand that overload resolution is painfully complicated already and I wouldn't suggest modifying any existing rules. I'd suggest that the "only" (almost as bas as "just") change would be that if two overloads are equally good and one has params T[] and the other has params Span<T> that the compiler would prefer the latter. This would only apply if otherwise it would result in a compile-time error. I don't see this as any different to how the compiler now considers generic type constraints with overload resolution.

I believe that API authors can reasonably migrate as @tannergooding has suggested.

I disagree. The BCL can't reasonably migrate due to downlevel compiler compatibility. The same would be true with any larger project with a wide developer base. Those projects would all either be left on T[] or they would have to define entirely new APIs just to consume this language feature. I see no reason to want to allow that hurdle to exist.

[popcatalin81]

I believe that API authors can reasonably migrate as @tannergooding has suggested

mscorlib cannot use that. String.Format cannot do that, Console.Writeline can’t do it. All of these would want to take advantage of params span

[popcatalin81]

These are the types and methods from mscorlib which have params array parameters:

Activator.CreateInstance (1 overloads)
AppDomain.CreateDomain (1 overloads)
AppDomain.ExecuteAssemblyByName (4 overloads)
Array.CreateInstance (2 overloads)
Array.GetValue (2 overloads)
Array.SetValue (2 overloads)
BCLDebug.Log (1 overloads)
BCLDebug.Trace (2 overloads)
CancellationTokenSource.CreateLinkedTokenSource (1 overloads)
Console.Write (1 overloads)
Console.WriteLine (1 overloads)
CryptoConfig.AddAlgorithm (1 overloads)
CryptoConfig.AddOID (1 overloads)
CryptoConfig.CreateFromName (1 overloads)
Delegate.Combine (1 overloads)
Delegate.DynamicInvoke (76 overloads)
Environment.GetResourceString (1 overloads)
Environment.GetRuntimeResourceString (1 overloads)
FormattableStringFactory.Create (1 overloads)
GenericTypeParameterBuilder.MakeGenericType (1 overloads)
GenericTypeParameterBuilder.SetInterfaceConstraints (1 overloads)
InternalRemotingServices.RemotingTrace (1 overloads)
InternalRM.InfoSoap (1 overloads)
InternalST.InfoSoap (1 overloads)
InternalST.Soap (1 overloads)
MetadataToken.IsTokenOfType (1 overloads)
MethodBuilder.DefineGenericParameters (1 overloads)
MethodBuilder.MakeGenericMethod (1 overloads)
MethodBuilder.SetParameters (1 overloads)
MethodBuilderInstantiation.MakeGenericMethod (1 overloads)
MethodInfo.MakeGenericMethod (4 overloads)
MethodOnTypeBuilderInstantiation.MakeGenericMethod (1 overloads)
Parallel.Invoke (2 overloads)
Path.Combine (1 overloads)
RuntimeMethodInfo.MakeGenericMethod (1 overloads)
RuntimeType.MakeGenericType (2 overloads)
Statics.CreateInstance (1 overloads)
String.Concat (2 overloads)
String.Format (2 overloads)
String.Join (2 overloads)
String.Split (1 overloads)
String.Trim (1 overloads)
String.TrimEnd (1 overloads)
String.TrimStart (1 overloads)
StringBuilder.AppendFormat (2 overloads)
SyncTextWriter.Write (1 overloads)
SyncTextWriter.WriteLine (1 overloads)
Task.WaitAll (1 overloads)
Task.WaitAny (1 overloads)
Task.WhenAll (2 overloads)
Task.WhenAny (2 overloads)
TextWriter.Write (4 overloads)
TextWriter.WriteLine (4 overloads)
Type.MakeGenericType (5 overloads)
TypeBuilder.DefineGenericParameters (1 overloads)
TypeBuilder.MakeGenericType (1 overloads)
TypeBuilderInstantiation.MakeGenericType (1 overloads)

Some of these methods would greatly benefit having a params span<T> overload in high performance code, especially the following, but not limited to :

String.Concat (2 overloads)
String.Format (2 overloads)
String.Join (2 overloads)
String.Split (1 overloads)
String.Trim (1 overloads)
String.TrimEnd (1 overloads)
String.TrimStart (1 overloads)
StringBuilder.AppendFormat (2 overloads)
Task.WaitAll (1 overloads)
Task.WaitAny (1 overloads)
Task.WhenAll (2 overloads)
Task.WhenAny (2 overloads)
TextWriter.Write (4 overloads)
TextWriter.WriteLine (4 overloads)

[gafter]

@popcatlin81 A param overload that takes a Span<T> where T is a reference type has no improved characteristics over the overload that takes an array. The caller still needs to allocate an array because you cannot stackalloc an array of something that is or contains a reference. So I believe few if any of your proposed new overloads would be valuable.