roslyn icon indicating copy to clipboard operation
roslyn copied to clipboard
verified publisher icon dotnet

[C#]`string` and `char` constants are not embedded as literals in string interpolation

Open tats-u opened this issue 1 year ago • 12 comments

Version Used:

main (10 Jan 2024) in sharplab

Steps to Reproduce:

using System;
public class C {
    public string M() {
        const string a = "S";
        const int b = 1;
        const char c = 'C';
        return $"foo{a}bar{c}{b}";
    }
}

.NET: https://sharplab.io/#v2:EYLgtghglgdgNAExAagD4AEBMBGAsAKHQGYACLEgYRIG8CT6zT1sAGEgWQAoBKGuhgQGMA9jADOAFzKsSEEgF4SAIgDKSgNz8B9EeKmwpwBSWyb82hrsklBACwgAnG8YDkFF2Yv10AdhIASJQAzYWFqCABfYEdqQQjqYAiNLRIIggigA

.NET Framework: https://sharplab.io/#v2:EYLgHgbALANAJiA1AHwAICYCMBYAUKgZgAIMiBhIgbzyNpONUwAYiBZACgEoqa6+BjAPYA7AM4AXEsyIBDIgF4iAIgDKSgNy8+tIWMkBLYZOAKimTbm11dEovwAWMgE53TAcjJuLV2qgDsRAAkSgBmgoKUMgC+wM6U/FGUwFEaWkRReFFAA=

Imports System
Public Class C 
    Public Function M()  As string
        const a = "S"
        const b = 1
        const c = "C"c
        return $"foo{a}bar{c}{b}"
    End Function
End Class

https://sharplab.io/#v2:EYLgtghglgdgNAGxAN2HAJiA1AHwJJgAOA9gE4AuAzgAQDKAnpeQKZgCwAUAAoCuwCUAMbUAwggiUaI6p2pzqvfkOoAxHjEHkoxGNQCyACgCUcgII0mpWAHNZ8+4J1NqEagF5qAIlqe79uY4wzsDu1ACMfv6BzsIeniKegpH2pMzkPKS6ACSeAGbExADeEAC+wBCkhYIlhcAlvhz2AKIw6KrqmtownC1tYhKUnEA

Diagnostic Id:

N/A

Expected Behavior:

.NET:

using System.Diagnostics;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Security;
using System.Security.Permissions;

[assembly: CompilationRelaxations(8)]
[assembly: RuntimeCompatibility(WrapNonExceptionThrows = true)]
[assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints | DebuggableAttribute.DebuggingModes.EnableEditAndContinue | DebuggableAttribute.DebuggingModes.DisableOptimizations)]
[assembly: SecurityPermission(SecurityAction.RequestMinimum, SkipVerification = true)]
[assembly: AssemblyVersion("0.0.0.0")]
[module: UnverifiableCode]
[module: RefSafetyRules(11)]

public class C
{
    [NullableContext(1)]
    public string M()
    {
        DefaultInterpolatedStringHandler defaultInterpolatedStringHandler = new DefaultInterpolatedStringHandler(8, 1);
        defaultInterpolatedStringHandler.AppendLiteral("fooSbarC");
        defaultInterpolatedStringHandler.AppendFormatted(1);
        return defaultInterpolatedStringHandler.ToStringAndClear();
    }
}

.NET Framework:

using System;
using System.Diagnostics;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Security;
using System.Security.Permissions;
using Microsoft.CodeAnalysis;

[assembly: CompilationRelaxations(8)]
[assembly: RuntimeCompatibility(WrapNonExceptionThrows = true)]
[assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default | DebuggableAttribute.DebuggingModes.DisableOptimizations | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints | DebuggableAttribute.DebuggingModes.EnableEditAndContinue)]
[assembly: SecurityPermission(SecurityAction.RequestMinimum, SkipVerification = true)]
[assembly: AssemblyVersion("0.0.0.0")]
[module: UnverifiableCode]
[module: RefSafetyRules(11)]
namespace Microsoft.CodeAnalysis
{
    [CompilerGenerated]
    [Embedded]
    internal sealed class EmbeddedAttribute : Attribute
    {
    }
}
namespace System.Runtime.CompilerServices
{
    [CompilerGenerated]
    [Embedded]
    [AttributeUsage(AttributeTargets.Module, AllowMultiple = false, Inherited = false)]
    internal sealed class RefSafetyRulesAttribute : Attribute
    {
        public readonly int Version;

        public RefSafetyRulesAttribute(int P_0)
        {
            Version = P_0;
        }
    }
}
public class C
{
    public string M()
    {
        return string.Format("fooSbarC{0}", 1);
    }
}

VB:

using System.Diagnostics;
using System.Reflection;
using System.Runtime.CompilerServices;

[assembly: CompilationRelaxations(8)]
[assembly: RuntimeCompatibility(WrapNonExceptionThrows = true)]
[assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints | DebuggableAttribute.DebuggingModes.EnableEditAndContinue | DebuggableAttribute.DebuggingModes.DisableOptimizations)]
[assembly: AssemblyVersion("0.0.0.0")]

public class C
{
    public string M()
    {
        return string.Format("fooSbarC{0}", 1);
    }
}

Actual Behavior:

.NET:

using System.Diagnostics;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Security;
using System.Security.Permissions;

[assembly: CompilationRelaxations(8)]
[assembly: RuntimeCompatibility(WrapNonExceptionThrows = true)]
[assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints | DebuggableAttribute.DebuggingModes.EnableEditAndContinue | DebuggableAttribute.DebuggingModes.DisableOptimizations)]
[assembly: SecurityPermission(SecurityAction.RequestMinimum, SkipVerification = true)]
[assembly: AssemblyVersion("0.0.0.0")]
[module: UnverifiableCode]
[module: RefSafetyRules(11)]

public class C
{
    [NullableContext(1)]
    public string M()
    {
        DefaultInterpolatedStringHandler defaultInterpolatedStringHandler = new DefaultInterpolatedStringHandler(6, 3);
        defaultInterpolatedStringHandler.AppendLiteral("foo");
        defaultInterpolatedStringHandler.AppendFormatted("S");
        defaultInterpolatedStringHandler.AppendLiteral("bar");
        defaultInterpolatedStringHandler.AppendFormatted('C');
        defaultInterpolatedStringHandler.AppendFormatted(1);
        return defaultInterpolatedStringHandler.ToStringAndClear();
    }
}

.NET Framework:

using System;
using System.Diagnostics;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Security;
using System.Security.Permissions;
using Microsoft.CodeAnalysis;

[assembly: CompilationRelaxations(8)]
[assembly: RuntimeCompatibility(WrapNonExceptionThrows = true)]
[assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default | DebuggableAttribute.DebuggingModes.DisableOptimizations | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints | DebuggableAttribute.DebuggingModes.EnableEditAndContinue)]
[assembly: SecurityPermission(SecurityAction.RequestMinimum, SkipVerification = true)]
[assembly: AssemblyVersion("0.0.0.0")]
[module: UnverifiableCode]
[module: RefSafetyRules(11)]
namespace Microsoft.CodeAnalysis
{
    [CompilerGenerated]
    [Embedded]
    internal sealed class EmbeddedAttribute : Attribute
    {
    }
}
namespace System.Runtime.CompilerServices
{
    [CompilerGenerated]
    [Embedded]
    [AttributeUsage(AttributeTargets.Module, AllowMultiple = false, Inherited = false)]
    internal sealed class RefSafetyRulesAttribute : Attribute
    {
        public readonly int Version;

        public RefSafetyRulesAttribute(int P_0)
        {
            Version = P_0;
        }
    }
}
public class C
{
    public string M()
    {
        return string.Format("foo{0}bar{1}{2}", "S", 'C', 1);
    }
}

VB:

using System.Diagnostics;
using System.Reflection;
using System.Runtime.CompilerServices;

[assembly: CompilationRelaxations(8)]
[assembly: RuntimeCompatibility(WrapNonExceptionThrows = true)]
[assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints | DebuggableAttribute.DebuggingModes.EnableEditAndContinue | DebuggableAttribute.DebuggingModes.DisableOptimizations)]
[assembly: AssemblyVersion("0.0.0.0")]

public class C
{
    public string M()
    {
        return string.Format("foo{0}bar{1}{2}", "S", 'C', 1);
    }
}

tats-u avatar Jan 25 '24 09:01 tats-u

In contrast to this, the string concatenation operator (C#: + / VB: &) handles these types of constants well. This is one of the ways in which interpolated strings is inferior to the concatenation operator.

tats-u avatar Mar 06 '24 15:03 tats-u

Marking for compiler team to assign a buddy for the PR contribution.

arunchndr avatar Mar 06 '24 20:03 arunchndr

@333fred pretty sure this is "By Design" but want to let you confirm.

jaredpar avatar Mar 08 '24 01:03 jaredpar

No, this is not by design. We could adjust this behavior as an optimization.

333fred avatar Mar 08 '24 02:03 333fred

  • Should this optimization be suppressed in some cases, e.g. in lambda? (See https://github.com/dotnet/roslyn/pull/27411)
  • Should we accept const string = $"foo {'b'}ar"; in C# 12 or only future versions?
    • If this should be accepted only in future C#, do we provide this optimization for char constants without accepting the above statement?

tats-u avatar Mar 11 '24 15:03 tats-u

  1. That's not about lambdas in general, it's about expression trees. And yes, expression trees need to reflect the code as written.
  2. This would not be a language feature and should not be tied to language version. It should be entirely invisible to compiled code; it should just get marginally faster with no observable side effects.

333fred avatar Mar 11 '24 16:03 333fred

It's about expression trees. And yes, expression trees need to reflect the code as written.

I managed to consider them in my PR but it looks like some code is not sufficient quality.

This would not be a language feature and should not be tied to language version. It should be entirely invisible to compiled code; it should just get marginally faster with no observable side effects.

We will not have to wait for the next version of .NET in the fix for C#. This change will be appreciated in VB too because of the existences of vbLf, ChrW (especially e.g. ChrW(&H201C) & ChrW(&H201D)), and NameOf, but it can be postponed to the next version of .NET to be tested more thoroughly in preview versions.

tats-u avatar Mar 17 '24 14:03 tats-u

vblang's issue: https://github.com/dotnet/vblang/issues/369

tats-u avatar Mar 17 '24 21:03 tats-u

but it looks like some code is not sufficient quality.

Fixed. I do not think that I have to make additional changes except for appending additional tests now.

tats-u avatar Mar 18 '24 13:03 tats-u

My current PR for C# only aims at only interpolated strings that return string values (not other types of handlers or formattables).

If the code satisfies one of the following condition, I think it should be as is:

  • Third party (not in-BCL) custom handlers not in System. or IFormattable other than FormattableString
  • The constructor argument for IFormatProvider (2nd one) of BCL handlers is a class implementing ICustomFormatter
  • FormattableString other than an argument of FormattableString.Invariant (or .CurrentCulture)
    • In other forms it is difficult to detect calls for .GetArgument(s)
  • In an expression tree

tats-u avatar Apr 20 '24 12:04 tats-u

Should we add a new attribute to mark handlers as those that treat string and character variables in the same way as string literals?

tats-u avatar Apr 22 '24 14:04 tats-u

I've forgotten to paste the link to an alive PR here: https://github.com/dotnet/roslyn/pull/72308

tats-u avatar Jun 29 '24 15:06 tats-u