Use stackalloc in System.Xml.Xsl.Runtime.NumberFormatterBase.ConvertToAlphabetic #67003
Conversation
ghost
added
the
community-contribution
filipnavara
removed
the
community-contribution
| @@ -82,7 +82,7 @@ public static void ConvertToAlphabetic(StringBuilder sb, double val, char firstC | |||
| Debug.Assert(1 <= val && val <= MaxAlphabeticValue); | |||
| Debug.Assert(Math.Pow(totalChars, MaxAlphabeticLength) >= MaxAlphabeticValue); | |||
|
|
|||
| char[] letters = new char[MaxAlphabeticLength]; | |||
| Span<char> letters = stackalloc char[MaxAlphabeticLength]; | |||
There was a problem hiding this comment.
Unrelated to your change, but it'd be nice to use Math.DivRem below.
| @@ -93,7 +93,7 @@ public static void ConvertToAlphabetic(StringBuilder sb, double val, char firstC | |||
| number = quot; | |||
| } | |||
| letters[--idx] = (char)(firstChar + --number); | |||
| sb.Append(letters, idx, MaxAlphabeticLength - idx); | |||
| sb.Append(letters.Slice(idx, MaxAlphabeticLength - idx)); | |||
There was a problem hiding this comment.
Is the , MaxAlphabeticLength - idx part needed? It couldn't just be sb.Append(letters.Slice(id))?
There was a problem hiding this comment.
I think it is the same , just wanted to keep the code almost the same as before.
|
Tagging subscribers to this area: @dotnet/area-system-xml Issue DetailsAnother usage of new char[7] found and replaced with stackalloc
|
filipnavara
added
the
community-contribution
There was a problem hiding this comment.
LGTM I'd recommend to apply changes suggested by @stephentoub, please let me know if you plan to leave as is
|
@krwq I don't feel comfortable to change logic here as I cannot find a proper tests for this methods. |
|
@TrayanZapryanov I'll merge as is in that case. If you feel like making the change feel free to create follow up PR. For the tests I'm currently not sure - I'd probably put a breakpoint in that code path and start running all tests and see the stacktrace. Optionally just always throw an exception from that method and see which tests fail. |
Co-authored-by: Trayan Zapryanov <Traian.Zaprianov@docuware.com>
ghost
locked as resolved and limited conversation to collaborators
Another usage of new char[7] found and replaced with stackalloc