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

Updating generic math to support user-defined checked operators #67714

Merged
3 changes: 2 additions & 1 deletion eng/CodeAnalysis.src.globalconfig
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,8 @@ dotnet_diagnostic.IL3002.severity = warning
dotnet_diagnostic.SA0001.severity = none

# SA1000: Spacing around keywords
dotnet_diagnostic.SA1000.severity = warning
# suggestion until https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3478 is resolved
dotnet_diagnostic.SA1000.severity = suggestion

# SA1001: Commas should not be preceded by whitespace
dotnet_diagnostic.SA1001.severity = warning
Expand Down
28 changes: 14 additions & 14 deletions src/libraries/System.Private.CoreLib/src/System/Byte.cs
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ object IConvertible.ToType(Type type, IFormatProvider? provider)
/// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_Addition(TSelf, TOther)" />
static byte IAdditionOperators<byte, byte, byte>.operator +(byte left, byte right) => (byte)(left + right);

// /// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_CheckedAddition(TSelf, TOther)" />
// static byte IAdditionOperators<byte, byte, byte>.operator checked +(byte left, byte right) => checked((byte)(left + right));
/// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_CheckedAddition(TSelf, TOther)" />
static byte IAdditionOperators<byte, byte, byte>.operator checked +(byte left, byte right) => checked((byte)(left + right));
tannergooding marked this conversation as resolved.
Show resolved Hide resolved

//
// IAdditiveIdentity
Expand Down Expand Up @@ -381,8 +381,8 @@ object IConvertible.ToType(Type type, IFormatProvider? provider)
/// <inheritdoc cref="IDecrementOperators{TSelf}.op_Decrement(TSelf)" />
static byte IDecrementOperators<byte>.operator --(byte value) => --value;

// /// <inheritdoc cref="IDecrementOperators{TSelf}.op_CheckedDecrement(TSelf)" />
// static byte IDecrementOperators<byte>.operator checked --(byte value) => checked(--value);
/// <inheritdoc cref="IDecrementOperators{TSelf}.op_CheckedDecrement(TSelf)" />
static byte IDecrementOperators<byte>.operator checked --(byte value) => checked(--value);

//
// IDivisionOperators
Expand All @@ -391,8 +391,8 @@ object IConvertible.ToType(Type type, IFormatProvider? provider)
/// <inheritdoc cref="IDivisionOperators{TSelf, TOther, TResult}.op_Division(TSelf, TOther)" />
static byte IDivisionOperators<byte, byte, byte>.operator /(byte left, byte right) => (byte)(left / right);

// /// <inheritdoc cref="IDivisionOperators{TSelf, TOther, TResult}.op_CheckedDivision(TSelf, TOther)" />
// static byte IDivisionOperators<byte, byte, byte>.operator checked /(byte left, byte right) => checked((byte)(left / right));
/// <inheritdoc cref="IDivisionOperators{TSelf, TOther, TResult}.op_CheckedDivision(TSelf, TOther)" />
static byte IDivisionOperators<byte, byte, byte>.operator checked /(byte left, byte right) => (byte)(left / right);

//
// IEqualityOperators
Expand All @@ -411,8 +411,8 @@ object IConvertible.ToType(Type type, IFormatProvider? provider)
/// <inheritdoc cref="IIncrementOperators{TSelf}.op_Increment(TSelf)" />
static byte IIncrementOperators<byte>.operator ++(byte value) => ++value;

// /// <inheritdoc cref="IIncrementOperators{TSelf}.op_CheckedIncrement(TSelf)" />
// static byte IIncrementOperators<byte>.operator checked ++(byte value) => checked(++value);
/// <inheritdoc cref="IIncrementOperators{TSelf}.op_CheckedIncrement(TSelf)" />
static byte IIncrementOperators<byte>.operator checked ++(byte value) => checked(++value);

//
// IMinMaxValue
Expand Down Expand Up @@ -445,8 +445,8 @@ object IConvertible.ToType(Type type, IFormatProvider? provider)
/// <inheritdoc cref="IMultiplyOperators{TSelf, TOther, TResult}.op_Multiply(TSelf, TOther)" />
static byte IMultiplyOperators<byte, byte, byte>.operator *(byte left, byte right) => (byte)(left * right);

// /// <inheritdoc cref="IMultiplyOperators{TSelf, TOther, TResult}.op_CheckedMultiply(TSelf, TOther)" />
// static byte IMultiplyOperators<byte, byte, byte>.operator checked *(byte left, byte right) => checked((byte)(left * right));
/// <inheritdoc cref="IMultiplyOperators{TSelf, TOther, TResult}.op_CheckedMultiply(TSelf, TOther)" />
static byte IMultiplyOperators<byte, byte, byte>.operator checked *(byte left, byte right) => checked((byte)(left * right));

//
// INumber
Expand Down Expand Up @@ -937,8 +937,8 @@ public static bool TryCreate<TOther>(TOther value, out byte result)
/// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_Subtraction(TSelf, TOther)" />
static byte ISubtractionOperators<byte, byte, byte>.operator -(byte left, byte right) => (byte)(left - right);

// /// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
// static byte ISubtractionOperators<byte, byte, byte>.operator checked -(byte left, byte right) => checked((byte)(left - right));
/// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
static byte ISubtractionOperators<byte, byte, byte>.operator checked -(byte left, byte right) => checked((byte)(left - right));

//
// IUnaryNegationOperators
Expand All @@ -947,8 +947,8 @@ public static bool TryCreate<TOther>(TOther value, out byte result)
/// <inheritdoc cref="IUnaryNegationOperators{TSelf, TResult}.op_UnaryNegation(TSelf)" />
static byte IUnaryNegationOperators<byte, byte>.operator -(byte value) => (byte)(-value);

// /// <inheritdoc cref="IUnaryNegationOperators{TSelf, TResult}.op_CheckedUnaryNegation(TSelf)" />
// static byte IUnaryNegationOperators<byte, byte>.operator checked -(byte value) => checked((byte)(-value));
/// <inheritdoc cref="IUnaryNegationOperators{TSelf, TResult}.op_CheckedUnaryNegation(TSelf)" />
static byte IUnaryNegationOperators<byte, byte>.operator checked -(byte value) => checked((byte)(-value));

//
// IUnaryPlusOperators
Expand Down
28 changes: 14 additions & 14 deletions src/libraries/System.Private.CoreLib/src/System/Char.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,8 +1068,8 @@ public static int ConvertToUtf32(string s, int index)
/// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_Addition(TSelf, TOther)" />
static char IAdditionOperators<char, char, char>.operator +(char left, char right) => (char) (left + right);

// /// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_Addition(TSelf, TOther)" />
// static char IAdditionOperators<char, char, char>.operator checked +(char left, char right) => checked((char)(left + right));
/// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_Addition(TSelf, TOther)" />
static char IAdditionOperators<char, char, char>.operator checked +(char left, char right) => checked((char)(left + right));

//
// IAdditiveIdentity
Expand Down Expand Up @@ -1149,8 +1149,8 @@ public static int ConvertToUtf32(string s, int index)
/// <inheritdoc cref="IDecrementOperators{TSelf}.op_Decrement(TSelf)" />
static char IDecrementOperators<char>.operator --(char value) => --value;

// /// <inheritdoc cref="IDecrementOperators{TSelf}.op_CheckedDecrement(TSelf)" />
// static char IDecrementOperators<char>.operator checked --(char value) => checked(--value);
/// <inheritdoc cref="IDecrementOperators{TSelf}.op_CheckedDecrement(TSelf)" />
static char IDecrementOperators<char>.operator checked --(char value) => checked(--value);

//
// IDivisionOperators
Expand All @@ -1159,8 +1159,8 @@ public static int ConvertToUtf32(string s, int index)
/// <inheritdoc cref="IDivisionOperators{TSelf, TOther, TResult}.op_Division(TSelf, TOther)" />
static char IDivisionOperators<char, char, char>.operator /(char left, char right) => (char)(left / right);

// /// <inheritdoc cref="IDivisionOperators{TSelf, TOther, TResult}.op_CheckedDivision(TSelf, TOther)" />
// static char IDivisionOperators<char, char, char>.operator /(char left, char right) => checked((char)(left / right));
/// <inheritdoc cref="IDivisionOperators{TSelf, TOther, TResult}.op_CheckedDivision(TSelf, TOther)" />
static char IDivisionOperators<char, char, char>.operator checked /(char left, char right) => (char)(left / right);

//
// IEqualityOperators
Expand All @@ -1179,8 +1179,8 @@ public static int ConvertToUtf32(string s, int index)
/// <inheritdoc cref="IIncrementOperators{TSelf}.op_Increment(TSelf)" />
static char IIncrementOperators<char>.operator ++(char value) => ++value;

// /// <inheritdoc cref="IIncrementOperators{TSelf}.op_CheckedIncrement(TSelf)" />
// static char IIncrementOperators<char>.operator checked ++(char value) => checked(++value);
/// <inheritdoc cref="IIncrementOperators{TSelf}.op_CheckedIncrement(TSelf)" />
static char IIncrementOperators<char>.operator checked ++(char value) => checked(++value);

//
// IMinMaxValue
Expand Down Expand Up @@ -1213,8 +1213,8 @@ public static int ConvertToUtf32(string s, int index)
/// <inheritdoc cref="IMultiplyOperators{TSelf, TOther, TResult}.op_Multiply(TSelf, TOther)" />
static char IMultiplyOperators<char, char, char>.operator *(char left, char right) => (char)(left * right);

// /// <inheritdoc cref="IMultiplyOperators{TSelf, TOther, TResult}.op_CheckedMultiply(TSelf, TOther)" />
// static char IMultiplyOperators<char, char, char>.operator checked *(char left, char right) => checked((char)(left * right));
/// <inheritdoc cref="IMultiplyOperators{TSelf, TOther, TResult}.op_CheckedMultiply(TSelf, TOther)" />
static char IMultiplyOperators<char, char, char>.operator checked *(char left, char right) => checked((char)(left * right));

//
// INumber
Expand Down Expand Up @@ -1733,8 +1733,8 @@ static bool ISpanParsable<char>.TryParse(ReadOnlySpan<char> s, IFormatProvider?
/// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_Subtraction(TSelf, TOther)" />
static char ISubtractionOperators<char, char, char>.operator -(char left, char right) => (char)(left - right);

// /// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
// static char ISubtractionOperators<char, char, char>.operator checked -(char left, char right) => checked((char)(left - right));
/// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
static char ISubtractionOperators<char, char, char>.operator checked -(char left, char right) => checked((char)(left - right));

//
// IUnaryNegationOperators
Expand All @@ -1743,8 +1743,8 @@ static bool ISpanParsable<char>.TryParse(ReadOnlySpan<char> s, IFormatProvider?
/// <inheritdoc cref="IUnaryNegationOperators{TSelf, TResult}.op_UnaryNegation(TSelf)" />
static char IUnaryNegationOperators<char, char>.operator -(char value) => (char)(-value);

// /// <inheritdoc cref="IUnaryNegationOperators{TSelf, TResult}.op_CheckedUnaryNegation(TSelf)" />
// static char IUnaryNegationOperators<char, char>.operator checked -(char value) => checked((char)(-value));
/// <inheritdoc cref="IUnaryNegationOperators{TSelf, TResult}.op_CheckedUnaryNegation(TSelf)" />
static char IUnaryNegationOperators<char, char>.operator checked -(char value) => checked((char)(-value));

//
// IUnaryPlusOperators
Expand Down
12 changes: 6 additions & 6 deletions src/libraries/System.Private.CoreLib/src/System/DateTime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1511,8 +1511,8 @@ internal static bool TryCreate(int year, int month, int day, int hour, int minut
// IAdditionOperators
//

// /// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_Addition(TSelf, TOther)" />
// static DateTime IAdditionOperators<DateTime, TimeSpan, DateTime>.operator checked +(DateTime left, TimeSpan right) => checked(left + right);
/// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_Addition(TSelf, TOther)" />
static DateTime IAdditionOperators<DateTime, TimeSpan, DateTime>.operator checked +(DateTime left, TimeSpan right) => left + right;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting. DateTime.MaxValue + TimeSpan.FromMilliseconds(1) will throw an exception, regardless of checked. In a generic math context, that feels a little strange, but I'm not sure there's a better answer (as long as we continue to want these only-slightly-math-related types implementing these interfaces).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an area where we'll want to consider the implications of there now being user-defined checked operators for certain core types (like DateTime and similar).

One could imagine us "properly" supporting checked/unchecked contexts for these types now. One could also imagine us continuing to always throw on overflow.

checked(expr) doesn't necessarily mean that expr will throw on overflow; and unchecked(expr) doesn't necessarily mean that expr will not throw on overflow. It really depends on the type and what operations it decides to support.

We'll probably want to decide on guidance as well, including the implications of taking the behavioral break for already compiled dependents.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also have the option of saying:

DateTime always overflows when used as a concrete type but when used in a generic math context it properly respects checked/unchecked. We can always have the public APIs and the explicit interface implementations SxS, just needing to have API review in agreement that's overall good/desirable

Copy link
Member

@stephentoub stephentoub Apr 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Seems like we basically have four options:

  1. Don't implement the generic math interfaces on these types.
  2. Leave the behavior as is. Someone using these types in a generic context might experience exceptions for "overflow" (but not actually OverflowException) even when in unchecked.
  3. Implement the new unchecked operators on these types to not throw. This means someone using these in a generic context will have more consistent behavior regardless of the concrete type being used, but it yields an inconsistency between how the type behaves when using it directly vs generically.
  4. (3) and change the existing behavior of the existing operators to be non-throwing. That's a breaking change in that someone could easily be relying on that behavior to stop "bad things" from happening.

We don't need to decide now, but please ensure there's an issue open to track making a decision around this for .NET 7.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will log an issue tracking this before merging.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


//
// IAdditiveIdentity
Expand Down Expand Up @@ -1550,10 +1550,10 @@ internal static bool TryCreate(int year, int month, int day, int hour, int minut
// ISubtractionOperators
//

// /// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
// static DateTime ISubtractionOperators<DateTime, TimeSpan, DateTime>.operator checked -(DateTime left, TimeSpan right) => checked(left - right);
/// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
static DateTime ISubtractionOperators<DateTime, TimeSpan, DateTime>.operator checked -(DateTime left, TimeSpan right) => left - right;

// /// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
// static TimeSpan ISubtractionOperators<DateTime, DateTime, TimeSpan>.operator checked -(DateTime left, DateTime right) => checked(left - right);
/// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
static TimeSpan ISubtractionOperators<DateTime, DateTime, TimeSpan>.operator checked -(DateTime left, DateTime right) => left - right;
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -867,8 +867,8 @@ public static implicit operator DateTimeOffset(DateTime dateTime) =>
// IAdditionOperators
//

// /// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_Addition(TSelf, TOther)" />
// static DateTimeOffset IAdditionOperators<DateTimeOffset, TimeSpan, DateTimeOffset>.operator checked +(DateTimeOffset left, TimeSpan right) => checked(left + right);
/// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_Addition(TSelf, TOther)" />
static DateTimeOffset IAdditionOperators<DateTimeOffset, TimeSpan, DateTimeOffset>.operator checked +(DateTimeOffset left, TimeSpan right) => left + right;

//
// IAdditiveIdentity
Expand Down Expand Up @@ -906,10 +906,10 @@ public static implicit operator DateTimeOffset(DateTime dateTime) =>
// ISubtractionOperators
//

// /// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
// static DateTimeOffset ISubtractionOperators<DateTimeOffset, TimeSpan, DateTimeOffset>.operator checked -(DateTimeOffset left, TimeSpan right) => checked(left - right);
/// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
static DateTimeOffset ISubtractionOperators<DateTimeOffset, TimeSpan, DateTimeOffset>.operator checked -(DateTimeOffset left, TimeSpan right) => left - right;

// /// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
// static TimeSpan ISubtractionOperators<DateTimeOffset, DateTimeOffset, TimeSpan>.operator checked -(DateTimeOffset left, DateTimeOffset right) => checked(left - right);
/// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
static TimeSpan ISubtractionOperators<DateTimeOffset, DateTimeOffset, TimeSpan>.operator checked -(DateTimeOffset left, DateTimeOffset right) => left - right;
}
}
28 changes: 14 additions & 14 deletions src/libraries/System.Private.CoreLib/src/System/Decimal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1082,8 +1082,8 @@ object IConvertible.ToType(Type type, IFormatProvider? provider)
// IAdditionOperators
//

// /// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_Addition(TSelf, TOther)" />
// static decimal IAdditionOperators<decimal, decimal, decimal>.operator checked +(decimal left, decimal right) => checked(left + right);
/// <inheritdoc cref="IAdditionOperators{TSelf, TOther, TResult}.op_Addition(TSelf, TOther)" />
static decimal IAdditionOperators<decimal, decimal, decimal>.operator checked +(decimal left, decimal right) => left + right;

//
// IAdditiveIdentity
Expand All @@ -1096,22 +1096,22 @@ object IConvertible.ToType(Type type, IFormatProvider? provider)
// IDecrementOperators
//

// /// <inheritdoc cref="IDecrementOperators{TSelf}.op_CheckedDecrement(TSelf)" />
// static decimal IDecrementOperators<decimal>.operator checked --(decimal value) => checked(--value);
/// <inheritdoc cref="IDecrementOperators{TSelf}.op_CheckedDecrement(TSelf)" />
static decimal IDecrementOperators<decimal>.operator checked --(decimal value) => --value;

//
// IDivisionOperators
//

// /// <inheritdoc cref="IDivisionOperators{TSelf, TOther, TResult}.op_CheckedDivision(TSelf, TOther)" />
// static decimal IDivisionOperators<decimal, decimal, decimal>.operator checked /(decimal left, decimal right) => checked(left / right);
/// <inheritdoc cref="IDivisionOperators{TSelf, TOther, TResult}.op_CheckedDivision(TSelf, TOther)" />
static decimal IDivisionOperators<decimal, decimal, decimal>.operator checked /(decimal left, decimal right) => left / right;

//
// IIncrementOperators
//

// /// <inheritdoc cref="IIncrementOperators{TSelf}.op_CheckedIncrement(TSelf)" />
// static decimal IIncrementOperators<decimal>.operator checked ++(decimal value) => checked(++value);
/// <inheritdoc cref="IIncrementOperators{TSelf}.op_CheckedIncrement(TSelf)" />
static decimal IIncrementOperators<decimal>.operator checked ++(decimal value) => ++value;

//
// IMinMaxValue
Expand All @@ -1134,8 +1134,8 @@ object IConvertible.ToType(Type type, IFormatProvider? provider)
// IMultiplyOperators
//

// /// <inheritdoc cref="IMultiplyOperators{TSelf, TOther, TResult}.op_CheckedMultiply(TSelf, TOther)" />
// public static decimal operator checked *(decimal left, decimal right) => checked(left * right);
/// <inheritdoc cref="IMultiplyOperators{TSelf, TOther, TResult}.op_CheckedMultiply(TSelf, TOther)" />
public static decimal operator checked *(decimal left, decimal right) => left * right;

//
// INumber
Expand Down Expand Up @@ -1504,14 +1504,14 @@ public static bool TryCreate<TOther>(TOther value, out decimal result)
// ISubtractionOperators
//

// /// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
// static decimal ISubtractionOperators<decimal, decimal, decimal>.operator checked -(decimal left, decimal right) => checked(left - right);
/// <inheritdoc cref="ISubtractionOperators{TSelf, TOther, TResult}.op_CheckedSubtraction(TSelf, TOther)" />
static decimal ISubtractionOperators<decimal, decimal, decimal>.operator checked -(decimal left, decimal right) => left - right;

//
// IUnaryNegationOperators
//

// /// <inheritdoc cref="IUnaryNegationOperators{TSelf, TResult}.op_CheckedUnaryNegation(TSelf)" />
// static decimal IUnaryNegationOperators<decimal, decimal>.operator checked -(decimal value) => checked(-value);
/// <inheritdoc cref="IUnaryNegationOperators{TSelf, TResult}.op_CheckedUnaryNegation(TSelf)" />
static decimal IUnaryNegationOperators<decimal, decimal>.operator checked -(decimal value) => -value;
}
}
Loading