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?

[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.