dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.7k stars 3.98k forks source link

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

Open tats-u opened 5 months ago

tats-u commented 5 months ago

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 commented 3 months ago

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.

arkalyanms commented 3 months ago

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

jaredpar commented 3 months ago

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

333fred commented 3 months ago

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

tats-u commented 3 months ago
333fred commented 3 months ago
  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.
tats-u commented 3 months ago

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 commented 3 months ago

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

tats-u commented 3 months ago

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 commented 2 months ago

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:

tats-u commented 2 months ago

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 commented 3 days ago

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