roslyn: Confusing error when generic extension methods with `in this` and T struct not compiling.

Case: Using generic extension method with in this T and where T is a struct

Version Used: C# 7.2 (VS 2017 15.5.4)

Steps to Reproduce:

    public static class TestExtension
    {
        public static void TestDispose<T>(in this T thisArg) where T : struct, IDisposable
        {

        }
    }

Expected Behavior:

The extension method should compile.

Actual Behavior:

Error	CS8338	The first parameter of an 'in' extension method 'TestDispose' must be a value type.

About this issue

  • Original URL
  • State: open
  • Created 6 years ago
  • Reactions: 11
  • Comments: 24 (17 by maintainers)

Most upvoted comments

I do not think an error nor a warning is justifiable in this case. Generic in this is just as valid as a regular generic in parameter. Even though you might not want to call methods on the value directly, why should that stop you from declaring the method at all?

After all, you can do this without any issue:

private delegate void TestInDelegate<T>(in T val);
private delegate void TestRefDelegate<T>(ref T val);

public static void Test1<T>(in T val) where T : struct
{
    var del = (TestInDelegate<T>)Delegate.CreateDelegate(typeof(TestInDelegate<T>), ((TestRefDelegate<T>)Test2<T>).Method);
    del.Invoke(in val);
}

public static void Test2<T>(ref T val) where T : struct
{
    val = default;
}

Calling Test1 on a readonly value will happily reset it, and the same would be done had the parameter been in this T val. I know this abuses the runtime’s ignorance (anyway out and in should have been modreq from the beginning in my humble opinion), but the point is that even if you cannot do much with the in reference yourself, you can still pass it to another method that might, be it a smart dynamically generated method, a “const-casted” method like in my example, or an external method from an assembly compiled from a language that knows better how to deal with readonly types and values than C#.

So, there are 4 things that need to be done:

  1. in this T should be allowed. It works just as well (or just as bad) as in T and so the same rules should apply to it. The limitations of ref this T (value type or generic parameter constrained to a value type) are just as applicable.

  2. Calling interface methods on any in T parameter should produce a warning that a defensive copy is about to be produced. If the point of in is to prevent producing copies, then the programmer should be warned in the case when a copy is about to be produced by every instance call rather than just once when the whole method is called. At least until there are readonly interface methods available.

  3. readonly struct generic constraint, as suggested. Even though (in my opinion), readonly type attribute should have been accompanied by readonly for methods (and properties) from the beginning, it is the least that can be done to make efficient value-type generics viable.

  4. Disambiguate between this T and ref this T when calling an extension method. At the moment, when both overloads are present, it is an ambiguous call, but there is no way to select the specific overload. The ref one should be chosen if possible, and the normal one in all the other cases.

This is bydesign. At least now. Anything that you do with ‘this’ here will have to be an interface call and thus would result in a copy.

The pattern would be nearly always a bad idea - worse than ordinary byval extension.

This is not a first bug report though. I wonder if we should change the error message or just allow this.

But this works in 7.2?

public static class TestExtension
{
    public static void TestDispose<T>(ref this T thisArg) where T : struct, IDisposable
    {

    }
}

For in, maybe need a T : readonly struct constraint?

Also accessing fields would be ok, so maybe warning/error on calling a method/property rather than error for using in T struct?

What @IllidanS4 said.

Agree 100% and a million upvotes.

struct S : I { }
interface I { }

static class Ext
{
	static void A<T>(this in T s) where T : struct, I { } // error
	static void B<T>(this ref T s) where T : struct, I { } // ok
	static void C<T>(in T s) where T : struct, I { } // ok
	static void D(this in S s) { } // ok
}

I list up the cases. What makes me confused is If A is not allowed, why C is ok? Isn’t extension method just a syntax sugar?

@halter73 - the problem is not in the interface dispatch. The problem is that the methods on the interface have no way to say “I will not mutate the instance”.

Compiler must assume that interface members will mutate receivers. We do pass the receiver by reference to the call, so they certainly can. If the variable is readonly - and “in” parameter is readonly, C# compiler must make a local copy before making it a receiver of mutating call.

And such copy would have to happen every time any interface member is used on in parameter. You will end up with lots of copying.

Its only generic methods; non-generic methods allow both readonly structs and non-readonly structs.

It doesn’t make sense for generics and non-readonly structs as the only methods you can use are ones constrained by the interface and interfaces don’t have field access only properties so every use of a non-reaodnly struct will cause a copy, removing the purpose of in.

in with a non-readonly struct should probably give a warning if a method call or property access on it is performed and that it will perform a copy https://github.com/dotnet/corefxlab/pull/2097#discussion_r165849837