roslyn icon indicating copy to clipboard operation
roslyn copied to clipboard

Unnecessary defensive copies on enums passed by `in`

Open hamarb123 opened this issue 1 year ago • 2 comments

The compiler emits unnecessary defensive copies on enums passed by in.

Version Used:

Sharplab.io Also reproduces with the compiler included in the .NET 8.0.100 SDK

Steps to Reproduce:

using System;
public class C
{
    public void M<T1, T2>(in Enum1 value1, ref Enum1 value2, Enum1 value3, in T1 value4, in T2 value5) where T1 : Enum where T2 : struct, Enum
    {
        value1.ToString(); //defensive copy (unnecessary)
        value2.ToString(); //no defensive copy
        value3.ToString(); //no defensive copy
        value4.ToString(); //defensive copy (unnecessary)
        value5.ToString(); //defensive copy (unnecessary)
    }
}
public enum Enum1
{
}

Link

Expected Behavior:

None of the calls to .ToString() emit a defensive copy, since none is required for any of them.

Actual Behavior:

value1.ToString() emits a defensive copy, and similar for value4 and value5.

hamarb123 avatar Feb 09 '24 05:02 hamarb123

Hey, I could take this, but just to ask a clarification: is there any circumstances in which the defensive copy is of use here?

I would think no since I can't seem to find anything that incurs side effects on System.Enum, but I'm just asking to be sure.

ghost avatar Feb 22 '24 16:02 ghost

None of the default implementations of the methods on System.Enum, System.ValueType or System.Object mutate, furthermore the ECMA specification says that you cannot provide additional methods or overrides on enums (see II.22.37 - 33. If Extends = System.Enum (i.e., type is a user-defined Enum) then). Therefore, any member access on any value of type Enum is provably non-mutating (note that this is not the case for ValueType or Object in general, since you can override ToString and similar in structs).

hamarb123 avatar Feb 22 '24 20:02 hamarb123

None of the default implementations of the methods on System.Enum, System.ValueType or System.Object mutate, ...

It's true that they do not mutate today. This PR and issue is essentially saying that they can never mutate again in the future because it's baking in that assumption into codegen. Further there are more .NET runtimes than the ones you listed here: SPOT for example. Before going forward with this PR we'll need to square with the runtime team if they're comfortable with this change being made and essentially saying that all compliant runtimes cannot have mutating methods on enum.

@davidwrighton, @jjonescz

jaredpar avatar Feb 27 '24 17:02 jaredpar