Reduce Unsafe usage

src/Ramstack.Parsing/Collections/ArrayBuilder.cs

@@ -3,6 +3,20 @@ namespace Ramstack.Parsing.Collections;
 /// <summary>
 /// Represents an array builder for elements of type <typeparamref name="T"/>.
 /// </summary>
+/// <remarks>
+/// This structure is optimized for performance and relies on proper initialization.
+///
+/// Always initialize instances using one of the available constructors with the <c>new</c> keyword,
+/// including the parameterless constructor: <c>new ArrayBuilder&lt;T>()</c>.
+///
+/// Do not use <c>default(ArrayBuilder&lt;T>)</c> or similar patterns, as this results
+/// in an uninitialized instance with an invalid internal state,
+/// causing a <see cref="NullReferenceException"/> when methods are called.
+///
+/// This design choice was made to avoid unnecessary overhead and maintain control over
+/// internal state initialization in modern C# versions where parameterless constructors
+/// in structs are supported.
+/// </remarks>
 /// <typeparam name="T">The type of elements stored in the array builder.</typeparam>
 [DebuggerDisplay("Count = {Count}")]
 [DebuggerTypeProxy(typeof(ArrayBuilderDebugView<>))]
@@ -33,12 +47,10 @@ public readonly ref T this[int index]
         get
         {
             var array = _array;
-            if ((uint)index >= (uint)_count)
+            if ((uint)index >= (uint)_count || (uint)index >= (uint)array.Length)
                 ThrowHelper.ThrowArgumentOutOfRangeException();
 
-            return ref Unsafe.Add(
-                ref MemoryMarshal.GetArrayDataReference(array),
-                (nint)(uint)index);
+            return ref array[index];
         }
     }
 

src/Ramstack.Parsing/Literal.String.cs

@@ -128,16 +128,26 @@ private static int TryParseImpl(ReadOnlySpan<char> s, out TResult? value)
                 }
                 else
                 {
-                    if (index + 5 >= s.Length)
+                    var p = s.Slice(index);
+                    if (p.Length < 6)
                         goto FAIL;
 
-                    ref var r = ref Unsafe.AsRef(in s[index]);
-
-                    var v1 = (nint)Unsafe.Add(ref r, 2);
-                    var v2 = (nint)Unsafe.Add(ref r, 3);
-                    var v3 = (nint)Unsafe.Add(ref r, 4);
-                    var v4 = (nint)Unsafe.Add(ref r, 5);
-
+                    var v1 = (nint)p[2];
+                    var v2 = (nint)p[3];
+                    var v3 = (nint)p[4];
+                    var v4 = (nint)p[5];
+
+                    // We use Unsafe-methods here to index into the HexTable without bounds checks.
+                    //
+                    // The check 'if ((uint)(v1 | v2 | v3 | v4) <= 127)' guarantees that all four values
+                    // are within the range 0..127.
+                    //
+                    // This makes it safe to access HexTable[v1], HexTable[v2], HexTable[v3], and HexTable[v4]
+                    // without additional bounds checks, because HexTable has exactly 128 elements.
+                    // By using Unsafe.Add, we avoid the overhead of safety checks and improve performance.
+                    //
+                    // Once the JIT compiler can properly recognize and optimize this pattern, we may be able
+                    // to replace these unsafe calls with safe indexed access without performance loss.
                     if ((uint)(v1 | v2 | v3 | v4) > 127)
                         goto FAIL;
 

src/Ramstack.Parsing/Literal.UnicodeEscapeSequence.cs

@@ -51,6 +51,17 @@ public override bool TryParse(ref ParseContext context, out T value)
                 var v3 = (nint)s[4];
                 var v4 = (nint)s[5];
 
+                // We use Unsafe-methods here to index into the HexTable without bounds checks.
+                //
+                // The check 'if ((uint)(v1 | v2 | v3 | v4) <= 127)' guarantees that all four values
+                // are within the range 0..127.
+                //
+                // This makes it safe to access HexTable[v1], HexTable[v2], HexTable[v3], and HexTable[v4]
+                // without additional bounds checks, because HexTable has exactly 128 elements.
+                // By using Unsafe.Add, we avoid the overhead of safety checks and improve performance.
+                //
+                // Once the JIT compiler can properly recognize and optimize this pattern, we may be able
+                // to replace these unsafe calls with safe indexed access without performance loss.
                 if ((uint)(v1 | v2 | v3 | v4) <= 127)
                 {
                     ref var table = ref MemoryMarshal.GetReference(HexTable);

src/Ramstack.Parsing/ParseContext.cs

@@ -32,6 +32,8 @@ public readonly ReadOnlySpan<char> Remaining
             var source = _source;
             var length = source.Length - _position;
 
+            Debug.Assert(_source[_position..].Length == length);
+
             ref var reference = ref Unsafe.Add(
                 ref MemoryMarshal.GetReference(source),
                 (nint)(uint)_position);

src/Ramstack.Parsing/Parser.Repeat.cs

@@ -451,7 +451,8 @@ public override bool TryParse(ref ParseContext context, [NotNullWhen(true)] out
             while (s.Length != 0)
             {
                 var index = _searcher.IndexOfAnyExcept(s);
-                if (index >= 0)
+
+                if ((uint)index < (uint)s.Length)
                 {
                     count += index;
 
@@ -461,13 +462,21 @@ public override bool TryParse(ref ParseContext context, [NotNullWhen(true)] out
                     index++;
                     count++;
 
-                    Debug.Assert((uint)index <= (uint)s.Length);
-
-                    s = MemoryMarshal.CreateReadOnlySpan(
-                        ref Unsafe.Add(ref MemoryMarshal.GetReference(s), (nint)(uint)index),
-                        s.Length - index);
+                    // At this point, "index" is guaranteed to be within [0..s.Length].
+                    // While the Slice method performs its own bounds check and would throw if out of range,
+                    // our explicit check helps the JIT recognize that the call to Slice is safe.
+                    // This allows the JIT to eliminate the internal bounds check
+                    // and avoid generating unnecessary exception-handling code inside Slice.
+                    //
+                    // In other words, the check would happen anyway, but by placing it here explicitly,
+                    // we eliminate the need for exception code inside Slice itself.
+                    //
+                    // Even though logically (uint)index < (uint)s.Length implies (uint)index <= (uint)s.Length,
+                    // the JIT cannot currently prove this identity and therefore cannot optimize away
+                    // the redundant check in Slice.
 
-                    // s = s.Slice(index);
+                    if ((uint)index <= (uint)s.Length)
+                        s = s.Slice(index);
                 }
                 else
                 {
@@ -535,7 +544,7 @@ public override bool TryParse(ref ParseContext context, out Unit value)
             while (s.Length != 0)
             {
                 var index = _searcher.IndexOfAnyExcept(s);
-                if (index >= 0)
+                if ((uint)index < (uint)s.Length)
                 {
                     count += index;
 
@@ -545,13 +554,21 @@ public override bool TryParse(ref ParseContext context, out Unit value)
                     index++;
                     count++;
 
-                    Debug.Assert((uint)index <= (uint)s.Length);
-
-                    s = MemoryMarshal.CreateReadOnlySpan(
-                        ref Unsafe.Add(ref MemoryMarshal.GetReference(s), (nint)(uint)index),
-                        s.Length - index);
+                    // At this point, "index" is guaranteed to be within [0..s.Length].
+                    // While the Slice method performs its own bounds check and would throw if out of range,
+                    // our explicit check helps the JIT recognize that the call to Slice is safe.
+                    // This allows the JIT to eliminate the internal bounds check
+                    // and avoid generating unnecessary exception-handling code inside Slice.
+                    //
+                    // In other words, the check would happen anyway, but by placing it here explicitly,
+                    // we eliminate the need for exception code inside Slice itself.
+                    //
+                    // Even though logically (uint)index < (uint)s.Length implies (uint)index <= (uint)s.Length,
+                    // the JIT cannot currently prove this identity and therefore cannot optimize away
+                    // the redundant check in Slice.
 
-                    // s = s.Slice(index);
+                    if ((uint)index <= (uint)s.Length)
+                        s = s.Slice(index);
                 }
                 else
                 {