made read of nullability cache volatile and fixed other PR comments

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/CharKind.cs

@@ -29,11 +29,11 @@ internal static class CharKind
         /// <summary>Gets the next character kind from a context</summary>
         internal static uint Next(uint context) => context >> 3;
 
-        /// <summary>Creates the context of the previous and the next character kinds.</summary>
+        /// <summary>Encodes the pair (prevKind, nextKind) using 6 bits</summary>
         internal static uint Context(uint prevKind, uint nextKind) => (nextKind << 3) | prevKind;
 
-        /// <summary>Number of bits used to represet a character context</summary>
-        internal const int ContextBitWidth = 6;
+        /// <summary>Exclusive maximum context (limit) is 64 because a context uses bit-shifting where each kind needs 3 bits.</summary>
+        internal const int ContextLimit = 64;
 
         internal static string DescribePrev(uint i) => i switch
         {

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/SymbolicRegexNode.cs

@@ -13,9 +13,12 @@ namespace System.Text.RegularExpressions.Symbolic
     internal sealed class SymbolicRegexNode<S> where S : notnull
     {
         internal const string EmptyCharClass = "[]";
+        /// <summary>Some byte other than 0 to represent true</summary>
         internal const byte TrueByte = 1;
+        /// <summary>Some byte other than 0 to represent false</summary>
         internal const byte FalseByte = 2;
-        internal const byte UndefByte = 0;
+        /// <summary>The undefined value is the default value 0</summary>
+        internal const byte UndefinedByte = 0;
 
         internal readonly SymbolicRegexBuilder<S> _builder;
         internal readonly SymbolicRegexKind _kind;
@@ -26,7 +29,11 @@ internal sealed class SymbolicRegexNode<S> where S : notnull
         internal readonly SymbolicRegexNode<S>? _right;
         internal readonly SymbolicRegexSet<S>? _alts;
 
-        private byte[]? _nullabilityForContext;
+        /// <summary>
+        /// Caches nullability of this node for any given context (0 &lt;= context &lt; ContextLimit)
+        /// when _info.StartsWithSomeAnchor and _info.CanBeNullable are true. Otherwise the cache is null.
+        /// </summary>
+        private byte[]? _nullabilityCache;
 
         private S _startSet;
 
@@ -53,7 +60,7 @@ private SymbolicRegexNode(SymbolicRegexBuilder<S> builder, SymbolicRegexKind kin
             _info = info;
             _hashcode = ComputeHashCode();
             _startSet = ComputeStartSet();
-            _nullabilityForContext = info.StartsWithSomeAnchor && info.CanBeNullable ? new byte[1 << CharKind.ContextBitWidth] : null;
+            _nullabilityCache = info.StartsWithSomeAnchor && info.CanBeNullable ? new byte[CharKind.ContextLimit] : null;
         }
 
         private bool _isInternalizedUnion;
@@ -166,23 +173,27 @@ static void AppendToList(SymbolicRegexNode<S> concat, List<SymbolicRegexNode<S>>
         /// <param name="context">kind info for previous and next characters</param>
         internal bool IsNullableFor(uint context)
         {
-            if (!_info.StartsWithSomeAnchor)
-                return IsNullable;
-
-            if (!_info.CanBeNullable)
-                return false;
+            if (_nullabilityCache is null)
+            {
+                // if _nullabilityCache is null then IsNullable==CanBeNullable
+                // Observe that if IsNullable==true then CanBeNullable==true.
+                // but when the node does not start with an anchor
+                // and IsNullable==false then CanBeNullable==false.
+                return _info.IsNullable;
+            }
 
             if (!StackHelper.TryEnsureSufficientExecutionStack())
             {
                 return StackHelper.CallOnEmptyStack(IsNullableFor, context);
             }
 
-            Debug.Assert(_nullabilityForContext is not null && context < (1 << CharKind.ContextBitWidth));
+            Debug.Assert(context < CharKind.ContextLimit);
 
             // If nullablity has been computed for the given context then return it
-            if (_nullabilityForContext[context] != UndefByte)
+            byte b = Volatile.Read(ref _nullabilityCache[context]);
+            if (b != UndefinedByte)
             {
-                return _nullabilityForContext[context] == TrueByte;
+                return b == TrueByte;
             }
 
             // Otherwise compute the nullability recursively for the given context
@@ -246,16 +257,15 @@ internal bool IsNullableFor(uint context)
                     is_nullable = (CharKind.Next(context) & CharKind.StartStop) != 0;
                     break;
 
-                default: //SymbolicRegexKind.EndAnchorZRev:
+                default: // SymbolicRegexKind.EndAnchorZRev:
                          // EndAnchorZRev (rev(\Z)) anchor is nullable when the prev character is either the first Newline or Start
                          // note: CharKind.NewLineS == CharKind.Newline|CharKind.StartStop
                     Debug.Assert(_kind == SymbolicRegexKind.EndAnchorZRev);
                     is_nullable = (CharKind.Prev(context) & CharKind.StartStop) != 0;
                     break;
             }
 
-            Volatile.Write(ref _nullabilityForContext[context], is_nullable ? TrueByte : FalseByte);
-            }
+            Volatile.Write(ref _nullabilityCache[context], is_nullable ? TrueByte : FalseByte);
 
             return is_nullable;
         }