dotnet / csharplang

The official repo for the design of the C# programming language
11.49k stars 1.02k forks source link

Allow generic extension methods with 'this in T' and T struct to compile #3429

Open gerhard17 opened 4 years ago

gerhard17 commented 4 years ago

Overview I want generic extension methods with this in T parameter to compile.

The spec explicitly disallows this usage: "On the other hand in extension methods exist specifically to reduce implicit copying. However any use of an in T parameter will have to be done through an interface member. Since all interface members are considered mutating, any such use would require a copy. - Instead of reducing copying, the effect would be the opposite. Therefore in this T is not allowed when T is a generic type parameter regardless of constraints."

But not every use of an in T parameter will have to be done through an interface member.

  1. It could be used by another generic method with in T parameter.
  2. It can be finally consumed by an Unsafe.SizeOf<T>() or Unsafe.AsRef<T>() and converted to a managed pointer, which does not require any struct copy.

Motivation I have implemented an arbitrary precision floating point library designed for floats from 16 bytes up to a few hundred kbytes. The various floats are defined by a struct of desired size which must only implement the empty interface IFpFloatReadonlyStruct. No struct methods are implemented directly (beside a ToString() override).

All float structs share the same generic library code. The core functioniality is implemented in only 3 extension methods based on

static uint GetStructSizeInSlots<TFloat>(ref TFloat number) where TFloat : struct, IFpFloatReadonlyStruct {..}
static ref readonly uint GetLead<TFloat>(ref TFloat number) where TFloat : struct, IFpFloatReadonlyStruct {..}
static ref readonly uint FractionAt<TFloat>(ref TFloat number, int index) where TFloat : struct, IFpFloatReadonlyStruct {..}

These three base methods are using managed pointer API similar to the System.Runtime.CompilerServcies.Unsafe class.

These base methods are consumed by higher level arithmetic functions like Add(), Multiply(), Log() and so on.

The JIT compiler really does an incredible good job with function inlining and generic code expansion.

Unintended struct copy does not occure during this usage. But any compiler warning - when doing so - would be strongly wellcomed!

Problem At the moment every in TFloat argument passing is done by using ref. But using ref instead of in has following drawbacks:

  1. the per design readonly input arguments can be assigned and therefore accidentially changed
  2. cannot be used with readonly 'constants' (readonly fields or ref readonly returning properties / methods)

Current Behavior Compiling

public interface IFpFloatReadonlyStruct { /*empty*/ }

public readonly struct FpFloatStruct256 : IFpFloatReadonlyStruct {
  ...
}

public static class FpFloatStruct {

  [MethodImpl(MethodImplOptions.AggressiveInlining)]
  public static ref readonly uint FractionAt<TFloat>(this in TFloat number, int index)
    where TFloat : struct, IFpFloatReadonlyStruct {
    return ref Unsafe.Add(ref Unsafe.As<TFloat, uint>(ref Unsafe.AsRef(number)), index + 1);
  }

}

raises following error at the function definition

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

Desired Behavior The above code compiles.

Breaking Change No, because currently this is an error. But a compiler warning - when unintential struct copy occurs - is strongly recommended.

Remarks I can supply more detailed examples and use cases, when desired.

Please consider a change. BR Gerhard

gerhard17 commented 4 years ago

see also: https://github.com/dotnet/roslyn/issues/24601

jnm2 commented 4 years ago

Duplicate of https://github.com/dotnet/csharplang/issues/3331

@tannergooding in that thread:

Actually, it might be because you can't know if this is safe:

    public static string Q<T>(in this T foo) where T : struct
    {
        return foo.ToString();
    }

The compiler would need to pessimistically assume that ToString() could mutate and would have to insert a hidden copy and readonly members isn't something you can know from the generic context.

gerhard17 commented 4 years ago

@jnm2 You are right, but it is not a problem of the generic method signature, it is a problem with the usage inside. So a (performance) warning at the line of usage foo.ToString() would be wellcomed in this case.

gerhard17 commented 4 years ago

To be clear: In this case I have no direct problem regarding calling a method on a hidden copy of the struct.

What I want is that

public static class FpFloatStruct {

  // first parameter declared:  this in

  [MethodImpl(MethodImplOptions.AggressiveInlining)]
  public static ref readonly uint FractionAt<TFloat>(this in TFloat number, int index)
    where TFloat : struct, IFpFloatReadonlyStruct {
    return ref Unsafe.Add(ref Unsafe.As<TFloat, uint>(ref Unsafe.AsRef(number)), index + 1);
  }

}

// USAGE

// define any struct value inheriting IFpFloatReadonlyStruct 
FpFloatStruct256 value = …

// call as static method - works without hidden struct copy
uint digit1 = FpFloatStruct.FractionAt(value, 0);

// call as extension method - not possible today
uint digit2 = value.FractionAt(0);

Debug.Assert(digit1 == digit2);

should compile, like

public static class FpFloatStruct {

  // first parameter declared:  in

  [MethodImpl(MethodImplOptions.AggressiveInlining)]
  public static ref readonly uint FractionAt<TFloat>(in TFloat number, int index)
    where TFloat : struct, IFpFloatReadonlyStruct {
    return ref Unsafe.Add(ref Unsafe.As<TFloat, uint>(ref Unsafe.AsRef(number)), index + 1);
  }

}

// USAGE

// define any struct value inheriting IFpFloatReadonlyStruct 
FpFloatStruct256 value = …

// call as static method - works without hidden struct copy
uint digit1 = FpFloatStruct.FractionAt(value, 0);

already compiles today.

The generated IL code and native JIT code should be the same in both cases.

Besides the difference in

.custom instance void [mscorlib]System.Runtime.CompilerServices.ExtensionAttribute::.ctor() = (01 00 00 00)

of course.

This allows a much more smarter usage (true extension method) and is symetric to the simple static method (without this).

A warning when operating on a hidden copy of the parameter struct will be wellcomed equally in both cases.

gerhard17 commented 3 years ago

Any news on this topic?

CyrusNajmabadi commented 3 years ago

Any news on this topic?

News is posted on the form of comments.

gerhard17 commented 3 days ago

See also: https://github.com/dotnet/csharplang/issues/7696