Roslyn: Avoid String.Format when interpolated strings are statically known

[danmosemsft]

Version Used:
2.0.2-vspre-006949

Steps to Reproduce:
```c#
using System;
public class C {
public void M(string arg) {
var knownStatically = "boz";
Console.WriteLine($"{nameof(arg)} baz {knownStatically}");
}
}

[live repro](
https://sharplab.io/#v2:EYLgtghgzgLgpgJwDQxASwDZICYgNQA+AAgAwAERAjANwCwAUEQMwUBMZAwmQN4Nn8UWRACxkAsgAoq5CAgDmASh58BAgG6yyAawB2AewDuOgMowIMNAGMIGDAE8yAXjIAiAGZ69LuvVUCqAJwSACQu3DoQYHB6bhKyigC+ZMAQAF48uoYmZhbWtnYJLgo+qgkMCUA==)

**Expected Behavior**:
IL contains

    IL_0007: ldstr "arg baz boz"

**Actual Behavior**:
IL contains 

    IL_0007: ldstr "{0} baz {1}"
    IL_000c: ldstr "arg"
    IL_0011: ldloc.0
    IL_0012: call string [mscorlib]System.String::Format(string, object, object)

**Motivation**
String.Format is not a cheap operation. This can be hit in cases like asserts, eg.,
``` c#
Debug.Assert(Capacity == Length, $"{nameof(ExpandByABlock)} should only be called when there is no space left.");

The above causes a String.Format which news up a StringBuilder even if the condition is true. Similar patterns can exist in retail builds.

/cc @stephentoub

[sharwell]

This appears to be a duplicate of #6122 and/or #4678.

:memo: I'm not closing because maybe we have more information suggesting that this is a worthwhile area to invest.

[tannergooding]

I thought it was determine by @gafter this couldn't be done due to the possibility of custom runtimes and/or custom cultures.

That being said, this is one of the many optimizations that I wish we had a "please make this assumption and optimize appropriately" switch for.

I think last time it was brought up, it was mentioned that a general-purpose, post-compilation, IL optimization tool would be a better investment.

[stephentoub]

In this case, the const in question is a string; culture would not affect that (string.ToString() is string, regardless of culture). And a custom runtime? We're seriously concerned about a runtime that implements string.Format to have different results than does .NET Framework, .NET Core, and Mono?

[tannergooding]

@stephentoub, it isn't calling string.ToString() though. It is calling string.ToString(IFormattable) which can be affected by Culture (however, on netfx, netcore, and mono all just behaves the same as string.ToString()).

We're seriously concerned about a runtime that implements string.Format to have different results than does .NET Framework, .NET Core, and Mono?

That's what has come up on several proposals and issues. Due to the existence of custom runtimes, there are several optimizations and or assumptions that cannot be made.

[tannergooding]

I don't necessarily agree with it, its just what has been stated time and time again.

[stephentoub]

it isn't calling string.ToString() though. It is calling string.ToString(IFormattable) which can be affected by Culture

No, that's my point... there is no formatting be done as part of ToString, regardless of overload... ToString on a string just returns that string, regardless of culture. It's documented:
https://msdn.microsoft.com/en-us/library/29dxe1x2(v=vs.110).aspx

Returns this instance of String; no actual conversion is performed.

Due to the existence of custom runtimes, there are several optimizations and or assumptions that cannot be made.

And especially in this case, I think that's irrelevant. We're shooting ourselves in the feet by taking such a stance.

I realize you're just the messenger...

[danmosemsft]

This means we should scan for a new antipattern in code reviews, which I wasn't previously aware of. I would not have expected lines like this, even in release to allocate a StringBuilder even when the condition is false.
``` c#
Trace.WriteIf(someFlag, $"{nameof(someFlag)} should be true.");

I think most developers would not expect this to execute any code if the flag is not set.

This is safe and preferred:
```c#
Trace.WriteIf(someFlag, nameof(someFlag) + " should be true.");

[stephentoub]

This means we should scan for a new antipattern in code reviews

I consider any use of string interpolation forbidden in corefx on any code paths where we might care about perf. They're fine in asserts, in exceptions, etc., but this issue aside, they're far from cheap.

I would not have expected lines like this, even in release to allocate a StringBuilder

It's using string.Format. And string.Format uses StringBuilderCache, which will allocate a StringBuilder if it doesn't already have one.