roslyn: IDE0071 - removing ToString causes boxing of the int value

This issue has been moved from a ticket on Developer Community.

    public class Class1
    {
        public int Value { get; set; }

        public string GetValue() => $"Value:{Value}";
        public string GetValueWithToString() => $"Value:{Value.ToString()}";
    }

When interpolating an int value, if we don’t call ToString we are boxing the value. I would expect to not see the suggestion to remove the ToString call. Moreover by looking at the IL, in the GetValue() method there is string. Format call while in the GetValueWithToString() there is a string. Concat call. If we use a reference type, there is no boxing of course, but the underlying method change as well.

I agree that the syntax is more concise without the ToString but the behavior is different which I would not expect from a refactoring.

Original Comments

Visual Studio Feedback System on 4/24/2020, 00:15 AM:

We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.

Original Solutions

(no solutions)

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 7
Comments: 22 (20 by maintainers)

Most upvoted comments

Couldn’t the compiler add the .ToString() itself? Then the refactoring would be correct and it would “unrefactor” behind the scenes to the faster variant.

+10

benaadams on May 1, 2020

Opened issue for compiler to improve codegen https://github.com/dotnet/roslyn/issues/44168

benaadams on May 12, 2020

I’m pretty sure we would be able to take such a change (though it may need to go through LDM). We’re already planning on taking a change to allow string-interpolations to be constants in C#9. It would likely make sense to roll this into that or similar efforts.

CyrusNajmabadi on May 1, 2020

Ah right, the runtime will box the int.

Anyways, there are a lot of refactorings that change application performance (foreach to linq for example). I am not convinced that this is worth not offering to the user because of a compiler optimization detail. You should be able to use existing configuration mechanisms to disable this analysis in your codebase

jmarolf on Apr 28, 2020

Yep, and String.Format’s 2nd parameter is of type object, so if you pass a value type it’ll get boxed.

canton7 on Apr 28, 2020

My interpretation is this:

The code fix suggests code which is easier to read
The code fix suggests code which is functionally equivalent to the original
For most applications/users/scenarios, the performance difference between the two approaches is negligible
1. The cost of string allocations is likely higher than boxing, so if this did appear on a performance-sensitive path it would likely be part of a pooling or caching strategy anyway
2. Users who do not fall into this group are likely to be aware that they fall into such a category and disable this analyzer anyway
The compiler and/or runtime have been known in the past to optimize cases like those produced by the code fix

It seems the diagnostic and code fix are performing as expected for majority audiences. My recommendation would be filing a code generation feature request for either the C# compiler or the JIT over in dotnet/runtime to detect and optimize cases produced by this code fix.

sharwell on May 1, 2020

Thanks for the feedback! Disabling IDE0071 is OK but it will remove the suggestion for all cases, not only value types. It would be great to have to suggestion only for reference types but it’s not vital.

Disabling it works for me since I’m using the additional ToString only for value types anyway.

davidelettieri on Apr 28, 2020