Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INumber or INumeric interface for base number types #14723

Closed
MovGP0 opened this issue Jun 16, 2015 · 15 comments
Closed

INumber or INumeric interface for base number types #14723

MovGP0 opened this issue Jun 16, 2015 · 15 comments
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime
Milestone

Comments

@MovGP0
Copy link

MovGP0 commented Jun 16, 2015

I often run into the problem that I want to handle numeric base types in a common way.

Unfortunately, they are missing a common interface to express their similarities. Therefore I propose to create an INumber or INumeric interface on the following numeric base types:

  • System.Decimal
  • System.Double
  • System.Single
  • System.Int32
  • System.UInt32
  • System.Int64
  • System.UInt64

This interface should define common functionality, like +, -, *, /, etc.

@ellismg
Copy link
Contributor

ellismg commented Jun 16, 2015

We've explored designs like this internally in the past. One of the sticking points has been you can't define operators on interfaces today, requiring us to use methods like Add, Subtract, etc. Would you imagine BigInteger would also become INumeric?

@KrzysztofCwalina
Copy link
Member

cc: @CarolEidt
Carol might want to correct me if I am wrong, but I don't think the issue is in syntactical differences between operators and Add/Sub/etc methods. I think additional problem was perf. Operators are non-virtual and don't use the this pointer. We could devirtualized INumber calls but it would require work and might not ever get to the performance level of operators.

@CarolEidt
Copy link
Contributor

You are both right. The issue was defining operators on interfaces, and it is a non-syntactic issue. Operators are static methods (not instance) and interfaces cannot specify a static method. Thus, one can't specify the natural constraints for INumeric that one would like (i.e. that it have the expected operators). Other designs have added Add, Sub, etc. as instance methods, but that requires changing the definition of all the primitive types. I had proposed adding static interface methods some time ago for this very purpose, but it was considered to be too high cost and cross-cutting (language and runtime).

@grokys
Copy link

grokys commented Jun 16, 2015

This would indeed be incredibly useful, but I can understand the difficulties.

I wonder if it would be easier to add support for a where T : numeric to the compiler somehow? Or would this also require runtime changes?

@CarolEidt
Copy link
Contributor

There was some discussion about having operator constraints, e.g. where T: op_Addition (not necessarily this exact syntax). But even this requires runtime support, and doesn't address static properties (e.g. Zero) which would be desirable. It does address the issue of allowing subsets of operators, for different mathematical classes (e.g. Group, Ring, Field), though one could envision hierarchical interfaces for that purpose.

@mikedn
Copy link
Contributor

mikedn commented Jun 16, 2015

There are ways to use interfaces, operators and get good performance (to a certain limit depending on JIT's mood) but the solution suffers from at least one usability issue:

interface ICalculator<T> {
    T Add(T x, T y);
}

struct Int32Calculator : ICalculator<int> {
    public int Add(int x, int y) { return x + y; }
}

struct Number<T, TCalc> where TCalc : struct, ICalculator<T> {
    T value;
    public static implicit operator Number<T, TCalc>(T value) {
        return new Number<T, TCalc> { value = value };
    }
    public static implicit operator T(Number<T, TCalc> number) {
        return number.value;
    }
    static TCalc Calc {
        get { return default(TCalc); }
    }
    public static Number<T, TCalc> operator +(Number<T, TCalc> x, Number<T, TCalc> y) {
        return Calc.Add(x, y);
    }
}

static T Sum<T, TCalc>(T[] values) where TCalc : struct, ICalculator<T> {
    var sum = default(Number<T, TCalc>);
    foreach (var value in values)
        sum += value;
    return sum;
}

static void Main() {
    Console.WriteLine(Sum<int, Int32Calculator>(new[] { 1, 2, 3 }));
}

The code generated for Sum is identical to the code generated for a non-generic Sum(int[]) method (well, almost - there is a bug in JIT that needs a workaround that's not included in this code).

The issue is that to use Sum you have to specify the calculator type as a generic argument. Not exactly pretty and not easy to avoid. Maybe it would work if some kind of default generic arguments are added to the language but that's probably just as complicated as adding T : numeric.

@whoisj
Copy link

whoisj commented Jun 16, 2015

Similar discussion happening over in Rosyln Issues

@mikedn
Copy link
Contributor

mikedn commented Jun 16, 2015

@MaleDong's last example gives me an idea:

struct Number<T> {
    T value;
    public static implicit operator Number<T>(T value) {
        return new Number<T> { value = value };
    }
    public static implicit operator T(Number<T> number) {
        return number.value;
    }
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static Number<T> operator +(Number<T> x, Number<T> y) {
        if (typeof(T) == typeof(int)) {
            return (T)(object)((int)(object)x.value + (int)(object)y.value);
        }
        else {
            NotSupported();
            return default(Number<T>);
        }
    }
    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void NotSupported() {
        throw new NotSupportedException();
    }
}

static T Sum<T>(T[] values) {
    var sum = default(Number<T>);
    foreach (var value in values)
        sum += value;
    return sum;
}

static void Main() {
    Console.WriteLine(Sum(new[] { 1, 2, 3 }));
}

On x86 (yet to check RyuJIT) the code generated for Sum is almost OK:

006404AD 33 F6                xor         esi,esi  
006404AF 33 D2                xor         edx,edx  
006404B1 83 79 04 00          cmp         dword ptr [ecx+4],0  
006404B5 7E 10                jle         006404C7  
006404B7 8B 7C 91 08          mov         edi,dword ptr [ecx+edx*4+8]  
006404BB 03 F7                add         esi,edi  
006404BD 8B C6                mov         eax,esi  
006404BF 8B F0                mov         esi,eax  
006404C1 42                   inc         edx  
006404C2 39 51 04             cmp         dword ptr [ecx+4],edx  
006404C5 7F F0                jg          006404B7  
006404C7 8B C6                mov         eax,esi  

There are some warts like esi->eax->esi but it's possible that these no longer exist in RyuJIT. In any case, the code doesn't contains interface calls, boxing or other expensive operations.

However, this will likely "fail" as soon as you add more numeric types to operator+ as the code size will likely exceed the maximum code size the JIT is willing to inline.

@mellinoe
Copy link
Contributor

return (T)(object)((int)(object)x.value + (int)(object)y.value);

FWIW, this is the general pattern we use for the IL implementation of Vector (in System.Numerics.Vectors). You're right that we can avoid boxing/interface calls, etc, but also correct in that we probably can't get inlining to work well. For Vector we just rely on the compiler intrinsics to take over, anyways, so it has a special-case solution.

@mikedn
Copy link
Contributor

mikedn commented Jun 17, 2015

but also correct in that we probably can't get inlining to work well

I did a test with RyuJIT and inlining works well even if all the types requested by @MovGP0 are added to operator+. Unfortunately, the code generated for float and double is kablooey. I suppose that's fixable.

What I really don't like about this kind of implementation is that it is limited to certain numeric types, if the code wasn't written to work with BigInteger then it won't work and there's nothing a user can do about it except creating a different Number<T> struct. It's also a bit ugly that you can't block people from attempting to use Number<T> with unsupported Ts unless you add an interface to the supported numeric types so you have something to constrain T to.

@MovGP0
Copy link
Author

MovGP0 commented Jun 17, 2015

while static interfaces or code generation might be helpful, i think it might be much simpler to define methods like T Add(T left, T right) on a base interface. additional types could then be supported using generic extension methods on those base interfaces.

@terrajobst terrajobst self-assigned this Sep 29, 2015
@karelz
Copy link
Member

karelz commented Nov 18, 2016

We don't think using Add, etc. will be that helpful. Ideally we would like operators on interfaces, which requires language + library + runtime support.
Closing here - we should move this to User Voice per our issue guide.

@karelz karelz closed this as completed Nov 18, 2016
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@IanKemp
Copy link

IanKemp commented Feb 4, 2020

Since C# 8.0 allows interfaces with static members is it now possible to resurrect this?

Edit: I see that static interface methods doesn't yet include operators, but this is yet another reason to allow operators.

@IanKemp
Copy link

IanKemp commented Feb 5, 2020

@karelz as per dotnet/csharplang#52 (comment) and dotnet/csharplang#52 (comment) static operators on interfaces are now supported, so can we get this reopened?

@karelz
Copy link
Member

karelz commented Feb 5, 2020

If the language is capable of doing it (would be nice to verify it first), I am fine to reconsider it.
Rather than reopening ancient issues, I would prefer to open new one with more details (API proposal).

@ghost ghost locked as resolved and limited conversation to collaborators Jan 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime
Projects
None yet
Development

No branches or pull requests