Remove unsafe code from StringParser by EgorBo · Pull Request #125640 · dotnet/runtime

EgorBo · 2026-03-17T01:52:06Z

Replace pointer-based parsing with safe indexing and for loops in ParseNextInt32, ParseNextInt64, ParseNextUInt32, ParseNextUInt64.

Replace pointer-based parsing with safe indexing and for loops in ParseNextInt32, ParseNextInt64, ParseNextUInt32, ParseNextUInt64. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR removes unsafe pointer-based parsing from System.IO.StringParser numeric parsing helpers by switching to safe string indexing/for loops, with a small consistency fix around empty-component handling.

Changes:

Replaced fixed/pointer loops with safe indexed loops in ParseNextInt32/Int64/UInt32/UInt64.
Added/standardized empty-component validation (notably for ParseNextUInt64).

src/libraries/Common/src/System/IO/StringParser.cs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…d add empty-component tests - Use AsSpan slice in all parse methods so JIT can eliminate bounds checks - Add Theory test covering empty component -> InvalidDataException for all numeric parse methods Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR removes unsafe pointer-based parsing from System.IO.StringParser numeric parsing helpers, replacing it with safe ReadOnlySpan<char> indexing and for loops while keeping the same overflow/invalid-data behavior.

Changes:

Rewrote ParseNextInt32, ParseNextInt64, ParseNextUInt32, and ParseNextUInt64 to avoid fixed/pointers and instead parse via spans and indexing.
Added a new unit test intended to validate that empty components throw InvalidDataException for numeric parsing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/libraries/Common/src/System/IO/StringParser.cs	Replaces pointer-based numeric parsing loops with span/index-based parsing and explicit empty-component checks.
src/libraries/Common/tests/Tests/System/IO/StringParserTests.cs	Adds a theory to validate numeric parsers throw `InvalidDataException` on empty components.

src/libraries/Common/tests/Tests/System/IO/StringParserTests.cs

…anch Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

EgorBo · 2026-03-17T02:20:59Z

@MihuBot

Copilot

Pull request overview

This PR removes unsafe pointer-based parsing from StringParser’s numeric parsing APIs by switching to ReadOnlySpan<char> indexing/loops, and adds a regression test to ensure empty components throw InvalidDataException.

Changes:

Replaced pointer iteration with ReadOnlySpan<char> + for loops in ParseNextInt32/Int64/UInt32/UInt64.
Added a new xUnit theory covering empty components for all four numeric parse methods.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`src/libraries/Common/src/System/IO/StringParser.cs`	Converts numeric parsing implementations from unsafe pointer-based logic to safe span-based loops.
`src/libraries/Common/tests/Tests/System/IO/StringParserTests.cs`	Adds a theory to validate empty-component behavior for numeric parsers.

src/libraries/Common/src/System/IO/StringParser.cs

src/libraries/Common/tests/Tests/System/IO/StringParserTests.cs

Copilot

Pull request overview

This PR removes unsafe pointer-based parsing from StringParser numeric parsing methods by switching to ReadOnlySpan<char> indexing and for loops.

Changes:

Replaced pointer iteration with span indexing in ParseNextInt32 and ParseNextInt64.
Replaced pointer iteration with span indexing in ParseNextUInt32 and ParseNextUInt64.
Added explicit empty-span validation before parsing.

src/libraries/Common/src/System/IO/StringParser.cs

+            int startIndex = _startIndex;
+            int endIndex = _endIndex;
+            ReadOnlySpan<char> span = _buffer.AsSpan(startIndex, endIndex - startIndex);
+
+            if (span.IsEmpty)
+            {
+                ThrowForInvalidData();
+            }


src/libraries/Common/src/System/IO/StringParser.cs

            MoveNextOrFail();
-            if (_startIndex == _endIndex)
+
+            ReadOnlySpan<char> span = _buffer.AsSpan(_startIndex, _endIndex - _startIndex);


tannergooding · 2026-03-17T14:25:25Z

src/libraries/Common/src/System/IO/StringParser.cs


        /// <summary>Moves to the next component and parses it as an Int64.</summary>
-        public unsafe long ParseNextInt64()
+        public long ParseNextInt64()


Do we know why this is its own custom handler and not just reusing the shared number parsing code used by int/long?

When this was written, span didn't exist yet, and we didn't want to allocate strings for each component just to be able to parse them. Now that span exists and you can int/long.Parse from them, that should be doable.

dotnet-policy-service · 2026-03-17T14:44:32Z

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Remove unsafe code from StringParser

dd5276b

Replace pointer-based parsing with safe indexing and for loops in ParseNextInt32, ParseNextInt64, ParseNextUInt32, ParseNextUInt64. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 17, 2026 01:52

github-actions bot added the area-Meta label Mar 17, 2026

dotnet-policy-service bot assigned EgorBo Mar 17, 2026

Copilot started reviewing on behalf of EgorBo March 17, 2026 01:53 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

EgorBo and others added 2 commits March 17, 2026 02:57

Hoist fields to locals in StringParser parse methods

4fe274f

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 17, 2026 02:01

Copilot started reviewing on behalf of EgorBo March 17, 2026 02:02 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

src/libraries/Common/tests/Tests/System/IO/StringParserTests.cs Outdated Show resolved Hide resolved

src/libraries/Common/tests/Tests/System/IO/StringParserTests.cs Outdated Show resolved Hide resolved

Fix empty-component test: remove extra MoveNextOrFail, add default br…

a74125c

…anch Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

EgorBot mentioned this pull request Mar 17, 2026

Benchmarks for dotnet/runtime#125640 (for @EgorBo) EgorBot/Benchmarks#42

Open

MihuBot mentioned this pull request Mar 17, 2026

[JitDiff X64] [EgorBo] Remove unsafe code from StringParser MihuBot/runtime-utils#1820

Open

danmoseley requested a review from Copilot March 17, 2026 03:43

Copilot started reviewing on behalf of danmoseley March 17, 2026 03:44 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

src/libraries/Common/src/System/IO/StringParser.cs Show resolved Hide resolved

src/libraries/Common/tests/Tests/System/IO/StringParserTests.cs Outdated Show resolved Hide resolved

Update StringParserTests.cs

c5b7d94

EgorBo marked this pull request as ready for review March 17, 2026 14:14

Copilot AI review requested due to automatic review settings March 17, 2026 14:14

Copilot started reviewing on behalf of EgorBo March 17, 2026 14:18 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

tannergooding reviewed Mar 17, 2026

View reviewed changes

am11 added area-System.IO and removed area-Meta labels Mar 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove unsafe code from StringParser#125640

Remove unsafe code from StringParser#125640
EgorBo wants to merge 5 commits intodotnet:mainfrom
EgorBo:remove-unsafe-from-stringparser

EgorBo commented Mar 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

EgorBo commented Mar 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

tannergooding Mar 17, 2026

Uh oh!

stephentoub Mar 17, 2026 •

edited

Loading

Uh oh!

dotnet-policy-service bot commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

EgorBo commented Mar 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

EgorBo commented Mar 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

tannergooding Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

stephentoub Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dotnet-policy-service bot commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

stephentoub Mar 17, 2026 •

edited

Loading