-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Revert "Improve ArgumentOutOfRange exception in Math.Clamp" #119276
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
Conversation
This reverts commit 1d56b2a.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reverts changes from PR #118824 that improved ArgumentOutOfRange exceptions in Math.Clamp methods. The revert restores the original behavior of throwing a generic ArgumentException instead of the more specific ArgumentOutOfRangeException when the minimum value is greater than the maximum value in clamp operations.
Key changes:
- Reverts exception type from ArgumentOutOfRangeException back to ArgumentException for invalid min/max parameters
- Restores custom error message formatting instead of using the standard ArgumentOutOfRangeException helper
- Updates corresponding test expectations to match the reverted behavior
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/libraries/System.Runtime/tests/System.Runtime.Extensions.Tests/System/Math.cs | Updates test expectations to verify ArgumentException is thrown instead of ArgumentOutOfRangeException |
src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs | Reverts BigInteger.Clamp to use custom exception throwing with ArgumentException |
src/libraries/System.Runtime.Numerics/src/Resources/Strings.resx | Adds back the custom error message format for min/max validation |
src/libraries/System.Private.CoreLib/src/System/UInt128.cs | Reverts UInt128.Clamp to use Math.ThrowMinMaxException helper |
src/libraries/System.Private.CoreLib/src/System/Single.cs | Reverts Single.Clamp and ClampNative to use Math.ThrowMinMaxException helper |
src/libraries/System.Private.CoreLib/src/System/Math.cs | Reverts all Math.Clamp overloads and updates ThrowMinMaxException to throw ArgumentException |
src/libraries/System.Private.CoreLib/src/System/Int128.cs | Reverts Int128.Clamp to use Math.ThrowMinMaxException helper |
src/libraries/System.Private.CoreLib/src/System/Half.cs | Reverts Half.ClampNative to use Math.ThrowMinMaxException helper |
src/libraries/System.Private.CoreLib/src/System/Double.cs | Reverts Double.Clamp and ClampNative to use Math.ThrowMinMaxException helper |
Context: #118824 (comment) |
Tagging subscribers to this area: @dotnet/area-system-numerics |
If the implementation from the PR consistently threw when either |
The nuance around It fixes a scenario that's often important for perf to be consistent across the stack (whether The biggest potential problem is it's a 4 year old API (added in .NET 6) that people may be depending on it throwing by this point. |
…8824)" (dotnet#119276) This reverts commit 1d56b2a.
Reverts #118824