From b4a25734853234ac45900775caa67627f2519d70 Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Sat, 15 Jan 2022 08:16:10 -1000
Subject: [PATCH 01/11] Add tests, improve benchmarks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Current benchmark values:
[info] Benchmark                         Mode  Cnt   Score   Error  Units
[info] StringInBenchmarks.linearMatchIn  avgt    3  77.603 ± 0.529  ns/op
[info] StringInBenchmarks.radixMatchIn   avgt    3  59.675 ± 2.126  ns/op
---
 .../cats/parse/bench/StringInBench.scala      | 26 ++++++---
 .../src/main/scala/cats/parse/Parser.scala    | 28 +--------
 .../src/main/scala/cats/parse/RadixNode.scala | 57 +++++++++++++++++--
 .../test/scala/cats/parse/RadixNodeTest.scala | 44 ++++++++++++++
 4 files changed, 115 insertions(+), 40 deletions(-)

diff --git a/bench/src/main/scala/cats/parse/bench/StringInBench.scala b/bench/src/main/scala/cats/parse/bench/StringInBench.scala
index 93a8373b..004c940c 100644
--- a/bench/src/main/scala/cats/parse/bench/StringInBench.scala
+++ b/bench/src/main/scala/cats/parse/bench/StringInBench.scala
@@ -19,7 +19,8 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-package cats.parse.bench
+package cats.parse
+package bench
 
 import cats.parse.Parser
 import java.util.concurrent.TimeUnit
@@ -32,18 +33,19 @@ class StringInBenchmarks {
   val inputs =
     List("foofoo", "bar", "foobat", "foot", "foobar")
 
-  val stringIn = Parser.stringIn("foo" :: "bar" :: "foobar" :: "foofoo" :: "foobaz" :: Nil)
+  val stringsToMatch =     
+    "foobar" :: "foofoo" :: "foobaz" :: "foo" :: "bar" :: Nil
+  
+  val radixNode = RadixNode.fromStrings(stringsToMatch)
+
+  val stringIn = Parser.stringIn(stringsToMatch)
 
   val oneOf =
     Parser.oneOf(
-      Parser.string("foobar") ::
-        Parser.string("foobaz") ::
-        Parser.string("foofoo") ::
-        Parser.string("foo") ::
-        Parser.string("bar") ::
-        Nil
+      stringsToMatch.map { s => Parser.string(s) }
     )
 
+    /*
   @Benchmark
   def stringInParse(): Unit =
     inputs.foreach(stringIn.parseAll(_))
@@ -51,5 +53,13 @@ class StringInBenchmarks {
   @Benchmark
   def oneOfParse(): Unit =
     inputs.foreach(oneOf.parseAll(_))
+    */
+
+  @Benchmark
+  def radixMatchIn(): Unit =
+    inputs.foreach { s => radixNode.matchAt(s, 0) >= 0 }
 
+  @Benchmark
+  def linearMatchIn(): Unit =
+    inputs.foreach { s => stringsToMatch.exists(s.startsWith(_)) }
 }
diff --git a/core/shared/src/main/scala/cats/parse/Parser.scala b/core/shared/src/main/scala/cats/parse/Parser.scala
index 81aee6a7..9968055b 100644
--- a/core/shared/src/main/scala/cats/parse/Parser.scala
+++ b/core/shared/src/main/scala/cats/parse/Parser.scala
@@ -1536,7 +1536,7 @@ object Parser {
     if (cs.isEmpty) fail
     else {
       val ary = cs.toArray
-      java.util.Arrays.sort(ary)
+      Arrays.sort(ary)
       rangesFor(ary) match {
         case NonEmptyList((low, high), Nil) if low == Char.MinValue && high == Char.MaxValue =>
           anyChar
@@ -2444,33 +2444,9 @@ object Parser {
 
     final def stringIn[A](radix: RadixNode, all: SortedSet[String], state: State): Unit = {
       val str = state.str
-      val strLength = str.length
       val startOffset = state.offset
+      val lastMatch = radix.matchAt(str, startOffset)
 
-      var offset = startOffset
-      var tree = radix
-      var cont = offset < strLength
-      var lastMatch = -1
-
-      while (cont) {
-        val c = str.charAt(offset)
-        val idx = Arrays.binarySearch(tree.fsts, c)
-        if (idx >= 0) {
-          val prefix = tree.prefixes(idx)
-          // accept the prefix fo this character
-          if (str.startsWith(prefix, offset + 1)) {
-            val children = tree.children(idx)
-            offset += (prefix.length + 1)
-            tree = children
-            cont = offset < strLength
-            if (children.word) lastMatch = offset
-          } else {
-            cont = false
-          }
-        } else {
-          cont = false
-        }
-      }
       if (lastMatch < 0) {
         state.error = Eval.later(Chain.one(Expectation.OneOfStr(startOffset, all.toList)))
         state.offset = startOffset
diff --git a/core/shared/src/main/scala/cats/parse/RadixNode.scala b/core/shared/src/main/scala/cats/parse/RadixNode.scala
index fef53290..4e671a71 100644
--- a/core/shared/src/main/scala/cats/parse/RadixNode.scala
+++ b/core/shared/src/main/scala/cats/parse/RadixNode.scala
@@ -22,18 +22,57 @@
 package cats.parse
 
 import cats.data.NonEmptyList
-
+import java.util.Arrays
 import scala.annotation.tailrec
 
-private[parse] class RadixNode(
-    val fsts: Array[Char],
+private[parse] final class RadixNode(
+    protected val fsts: Array[Char],
     // the prefixes are the rest of the string after the fsts (not including the fsts Char)
-    val prefixes: Array[String],
-    val children: Array[RadixNode],
-    val word: Boolean
+    protected val prefixes: Array[String],
+    protected val children: Array[RadixNode],
+    protected val word: Boolean
 ) {
   override def toString(): String =
     s"RadixNode(${fsts.mkString("[", ", ", "]")}, ${children.mkString("[", ", ", "]")}, $word)"
+
+  /** If this matches, return the new offset, else return -1
+    *
+    * @param str
+    *   the string to match against this RadixNode
+    * @param offset
+    *   the initial offset
+    * @return
+    *   the new offset after a match, or -1
+    */
+  def matchAt(str: String, off: Int): Int = {
+    val strLength = str.length
+    var offset = off
+    var tree = this
+    var cont = offset < strLength
+    var lastMatch = -1
+
+    while (cont) {
+      val c = str.charAt(offset)
+      val idx = Arrays.binarySearch(tree.fsts, c)
+      if (idx >= 0) {
+        val prefix = tree.prefixes(idx)
+        // accept the prefix fo this character
+        if (str.startsWith(prefix, offset + 1)) {
+          val children = tree.children(idx)
+          offset += (prefix.length + 1)
+          tree = children
+          cont = offset < strLength
+          if (children.word) lastMatch = offset
+        } else {
+          cont = false
+        }
+      } else {
+        cont = false
+      }
+    }
+
+    lastMatch
+  }
 }
 
 private[parse] object RadixNode {
@@ -87,6 +126,12 @@ private[parse] object RadixNode {
         leaf
     }
 
+  def fromStrings(strs: Iterable[String]): RadixNode =
+    NonEmptyList.fromList(strs.toList.distinct.sorted) match {
+      case Some(nel) => fromSortedStrings(nel)
+      case None => leaf
+    }
+
   private val leaf = new RadixNode(Array.empty, Array.empty, Array.empty, true)
 
   final def commonPrefixLength(s1: String, s2: String): Int = {
diff --git a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
index 773022e3..737cb9f8 100644
--- a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
+++ b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
@@ -21,9 +21,17 @@
 
 package cats.parse
 
+import org.scalacheck.Prop
 import org.scalacheck.Prop.forAll
 
 class RadixNodeTest extends munit.ScalaCheckSuite {
+  val tests: Int = if (BitSetUtil.isScalaJs) 50 else 20000
+
+  override def scalaCheckTestParameters =
+    super.scalaCheckTestParameters
+      .withMinSuccessfulTests(tests)
+      .withMaxDiscardRatio(10)
+
   property("commonPrefixLength is consistent") {
     forAll { (s1: String, s2: String) =>
       val len = RadixNode.commonPrefixLength(s1, s2)
@@ -36,6 +44,12 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
     }
   }
 
+  property("commonPrefixLength is commutative") {
+    forAll { (s1: String, s2: String) =>
+      assertEquals(RadixNode.commonPrefixLength(s1, s2), RadixNode.commonPrefixLength(s2, s1))
+    }
+  }
+
   property("commonPrefixLength(s, s + r) == s.length") {
     forAll { (s: String, r: String) =>
       assert(RadixNode.commonPrefixLength(s, s + r) == s.length)
@@ -53,4 +67,34 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
       assertEquals(RadixNode.commonPrefixLength(s, r), RadixNode.commonPrefixLength(r, s))
     }
   }
+
+  property("If we match, then string is in the set") {
+    def law(ss0: List[String], target: String): Prop = {
+      val ss = ss0.filter(_.nonEmpty)
+      val radix = RadixNode.fromStrings(ss)
+      assertEquals(radix.matchAt(target, 0) >= 0, ss.exists(target.startsWith(_)))
+    }
+
+    val p1 = forAll { (ss: List[String], head: Char, tail: String) =>
+      val target = s"$head$tail"
+      law(ss, target)
+    }
+
+    val regressions =
+      (List("噈"), s"噈\u0000") ::
+        Nil
+
+    regressions.foldLeft(p1) { case (p, (ss, t)) => p && law(ss, t) }
+  }
+
+  property("If we match everything in the set") {
+    forAll { (ss0: List[String], head: Char, tail: String) =>
+      val s1 = s"$head$tail"
+      val ss = s1 :: ss0
+      val radix = RadixNode.fromStrings(ss)
+      ss.foreach { target =>
+        assert((radix.matchAt(target, 0) >= 0) || target.isEmpty)
+      }
+    }
+  }
 }

From 92adcf867b4463befddc8c76a71f8141fadeeb99 Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Sat, 15 Jan 2022 11:22:08 -1000
Subject: [PATCH 02/11] use a hashing rather than sorting stategy

---
 .../src/main/scala/cats/parse/RadixNode.scala | 155 +++++++++---------
 .../test/scala/cats/parse/RadixNodeTest.scala |  29 +++-
 2 files changed, 106 insertions(+), 78 deletions(-)

diff --git a/core/shared/src/main/scala/cats/parse/RadixNode.scala b/core/shared/src/main/scala/cats/parse/RadixNode.scala
index 4e671a71..082689d0 100644
--- a/core/shared/src/main/scala/cats/parse/RadixNode.scala
+++ b/core/shared/src/main/scala/cats/parse/RadixNode.scala
@@ -22,18 +22,24 @@
 package cats.parse
 
 import cats.data.NonEmptyList
-import java.util.Arrays
+import cats.kernel.Semilattice
 import scala.annotation.tailrec
 
+import cats.syntax.all._
+
 private[parse] final class RadixNode(
-    protected val fsts: Array[Char],
+    protected val matched: String,
+    protected val bitMask: Int,
     // the prefixes are the rest of the string after the fsts (not including the fsts Char)
     protected val prefixes: Array[String],
-    protected val children: Array[RadixNode],
-    protected val word: Boolean
+    protected val children: Array[RadixNode]
 ) {
-  override def toString(): String =
-    s"RadixNode(${fsts.mkString("[", ", ", "]")}, ${children.mkString("[", ", ", "]")}, $word)"
+  override def toString(): String = {
+    def list[A](ary: Array[A]): String = ary.mkString("[", ", ", "]")
+    val ps = list(prefixes)
+    val cs = list(children)
+    s"RadixNode($matched, $bitMask, $ps, $cs)"
+  }
 
   /** If this matches, return the new offset, else return -1
     *
@@ -44,95 +50,81 @@ private[parse] final class RadixNode(
     * @return
     *   the new offset after a match, or -1
     */
-  def matchAt(str: String, off: Int): Int = {
-    val strLength = str.length
-    var offset = off
-    var tree = this
-    var cont = offset < strLength
-    var lastMatch = -1
+  def matchAt(str: String, off: Int): Int =
+    matchAtOrNull(str, off) match {
+      case null => -1
+      case nonNull => nonNull.length
+    }
 
-    while (cont) {
+  @tailrec
+  final def matchAtOrNull(str: String, offset: Int): String =
+    if (offset < str.length)  {
       val c = str.charAt(offset)
-      val idx = Arrays.binarySearch(tree.fsts, c)
-      if (idx >= 0) {
-        val prefix = tree.prefixes(idx)
+      // this is a hash of c
+      val idx = c.toInt & bitMask
+      val prefix = prefixes(idx)
+      if (prefix ne null) {
         // accept the prefix fo this character
-        if (str.startsWith(prefix, offset + 1)) {
-          val children = tree.children(idx)
-          offset += (prefix.length + 1)
-          tree = children
-          cont = offset < strLength
-          if (children.word) lastMatch = offset
+        if (str.regionMatches(offset, prefix, 0, prefix.length)) {
+          children(idx).matchAtOrNull(str, offset + prefix.length)
         } else {
-          cont = false
+          matched
         }
       } else {
-        cont = false
+        matched
       }
     }
-
-    lastMatch
-  }
+    else {
+      matched
+    }
 }
 
 private[parse] object RadixNode {
-  @tailrec
-  private def groupByNonEmptyPrefix(
-      keys: List[String],
-      prefix: String,
-      current: NonEmptyList[String],
-      acc: List[(Char, String, NonEmptyList[String])]
-  ): List[(Char, String, NonEmptyList[String])] =
-    keys match {
-      case key :: keys =>
-        val prefixSize = commonPrefixLength(prefix, key)
-        if (prefixSize == 0) {
-          // no common prefix, group current suffixes together sorted again
-          groupByNonEmptyPrefix(
-            keys,
-            key,
-            NonEmptyList.one(key),
-            (prefix(0), prefix.tail, current.map(_.drop(prefix.size)).reverse) :: acc
-          )
-        } else {
-          // clip the prefix to the length, and continue
-          groupByNonEmptyPrefix(keys, prefix.take(prefixSize), key :: current, acc)
-        }
-      case Nil =>
-        (prefix(0), prefix.tail, current.map(_.drop(prefix.size)).reverse) :: acc
+  private val emptyStringArray = new Array[String](1)
+  private val emptyChildrenArray = new Array[RadixNode](1)
+
+  private def fromNonEmpty(prefix: String, nonEmpty: NonEmptyList[String]): RadixNode = {
+    val nonEmpties = nonEmpty.toList.filter(_.nonEmpty)
+    val headKeys = nonEmpties.iterator.map(_.head).toSet
+
+    @tailrec
+    def findBitMask(b: Int): Int =
+      if (b == 0xffff) b // biggest it can be
+      else {
+        val allDistinct = headKeys.iterator.map { c => c.toInt & b }.toSet.size == headKeys.size
+        if (allDistinct) b
+        else findBitMask((b << 1) | 1)
+      }
+
+    val bitMask = findBitMask((1 << Integer.highestOneBit(headKeys.size)) - 1)
+    val branching = bitMask + 1
+    val prefixes = new Array[String](branching)
+    val children = new Array[RadixNode](branching)
+    val tree = nonEmpties.groupByNel { s => (s.head.toInt & bitMask) }
+    tree.foreach { case (idx, strings) =>
+      // strings is a NonEmptyList[String] which all start with the same char
+      val prefix1 = strings.reduce(commonPrefixSemilattice)
+      val plen = prefix1.length
+      val node = fromNonEmpty(prefix + prefix1, strings.map(_.drop(plen)))
+      prefixes(idx) = prefix1
+      children(idx) = node
     }
 
+    // If nonEmpty contains the empty string, we have a valid prefix
+    val thisPrefix = if (nonEmpty.exists(_.isEmpty)) prefix else null
+    new RadixNode(thisPrefix, bitMask, prefixes, children)
+  }
+
   def fromSortedStrings(strings: NonEmptyList[String]): RadixNode =
-    NonEmptyList.fromList(strings.filter(_.nonEmpty)) match {
-      case Some(nonEmpty) =>
-        val grouped =
-          groupByNonEmptyPrefix(
-            nonEmpty.tail,
-            nonEmpty.head,
-            NonEmptyList.one(nonEmpty.head),
-            Nil
-          ).reverse
-            .map { case (fst, prefix, v) => (fst, prefix, fromSortedStrings(v)) }
-
-        val (fsts, prefixes, children) = grouped.unzip3
-
-        new RadixNode(
-          fsts.toArray,
-          prefixes.toArray,
-          children.toArray,
-          nonEmpty.size < strings.size
-        )
-      case None =>
-        leaf
-    }
+    fromNonEmpty("", strings)
 
   def fromStrings(strs: Iterable[String]): RadixNode =
-    NonEmptyList.fromList(strs.toList.distinct.sorted) match {
+    NonEmptyList.fromList(strs.toList.distinct) match {
       case Some(nel) => fromSortedStrings(nel)
-      case None => leaf
+      case None => empty
     }
 
-  private val leaf = new RadixNode(Array.empty, Array.empty, Array.empty, true)
+  private val empty = new RadixNode(null, 0, emptyStringArray, emptyChildrenArray)
 
   final def commonPrefixLength(s1: String, s2: String): Int = {
     val len = Integer.min(s1.length, s2.length)
@@ -148,4 +140,15 @@ private[parse] object RadixNode {
 
     idx
   }
+
+  val commonPrefixSemilattice: Semilattice[String] =
+    new Semilattice[String] {
+      def combine(x: String, y: String): String = {
+        val l = commonPrefixLength(x, y)
+        if (l == 0) ""
+        else if (l == x.length) x
+        else if (l == y.length) y
+        else x.take(l)
+      }
+    }
 }
diff --git a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
index 737cb9f8..df594f3b 100644
--- a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
+++ b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
@@ -72,7 +72,8 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
     def law(ss0: List[String], target: String): Prop = {
       val ss = ss0.filter(_.nonEmpty)
       val radix = RadixNode.fromStrings(ss)
-      assertEquals(radix.matchAt(target, 0) >= 0, ss.exists(target.startsWith(_)))
+      val matchLen = radix.matchAt(target, 0)
+      assertEquals(matchLen >= 0, ss.exists(target.startsWith(_)), s"ss=$ss, ss.size = ${ss.size}, matchLen=$matchLen, radix=$radix")
     }
 
     val p1 = forAll { (ss: List[String], head: Char, tail: String) =>
@@ -87,7 +88,13 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
     regressions.foldLeft(p1) { case (p, (ss, t)) => p && law(ss, t) }
   }
 
-  property("If we match everything in the set") {
+  property("fromString(Nil) matches nothing") {
+    forAll { (s: String) =>
+      assert(RadixNode.fromStrings(Nil).matchAt(s, 0) < 0)
+    }
+  }
+
+  property("we match everything in the set") {
     forAll { (ss0: List[String], head: Char, tail: String) =>
       val s1 = s"$head$tail"
       val ss = s1 :: ss0
@@ -97,4 +104,22 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
       }
     }
   }
+
+  property("commonPrefix is associative") {
+    val sl = RadixNode.commonPrefixSemilattice
+    forAll { (s0: String, s1: String, s2: String) =>
+      val left = sl.combine(sl.combine(s0, s1), s2)
+      val right = sl.combine(s0, sl.combine(s1, s2))
+      assertEquals(left, right)
+    }
+  }
+
+  property("commonPrefix commutes") {
+    val sl = RadixNode.commonPrefixSemilattice
+    forAll { (s0: String, s1: String) =>
+      val left = sl.combine(s0, s1)
+      val right = sl.combine(s1, s0)
+      assertEquals(left, right)
+    }
+  }
 }

From 9d814f4c9ba5cf5c074e5149981e753354ff59f8 Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Sat, 15 Jan 2022 14:15:39 -1000
Subject: [PATCH 03/11] improve tests

---
 .../cats/parse/bench/StringInBench.scala      |  6 +-
 .../src/main/scala/cats/parse/RadixNode.scala | 99 ++++++++++---------
 .../test/scala/cats/parse/RadixNodeTest.scala | 68 +++++++++++--
 3 files changed, 115 insertions(+), 58 deletions(-)

diff --git a/bench/src/main/scala/cats/parse/bench/StringInBench.scala b/bench/src/main/scala/cats/parse/bench/StringInBench.scala
index 004c940c..924f06a9 100644
--- a/bench/src/main/scala/cats/parse/bench/StringInBench.scala
+++ b/bench/src/main/scala/cats/parse/bench/StringInBench.scala
@@ -33,9 +33,9 @@ class StringInBenchmarks {
   val inputs =
     List("foofoo", "bar", "foobat", "foot", "foobar")
 
-  val stringsToMatch =     
+  val stringsToMatch =
     "foobar" :: "foofoo" :: "foobaz" :: "foo" :: "bar" :: Nil
-  
+
   val radixNode = RadixNode.fromStrings(stringsToMatch)
 
   val stringIn = Parser.stringIn(stringsToMatch)
@@ -45,7 +45,6 @@ class StringInBenchmarks {
       stringsToMatch.map { s => Parser.string(s) }
     )
 
-    /*
   @Benchmark
   def stringInParse(): Unit =
     inputs.foreach(stringIn.parseAll(_))
@@ -53,7 +52,6 @@ class StringInBenchmarks {
   @Benchmark
   def oneOfParse(): Unit =
     inputs.foreach(oneOf.parseAll(_))
-    */
 
   @Benchmark
   def radixMatchIn(): Unit =
diff --git a/core/shared/src/main/scala/cats/parse/RadixNode.scala b/core/shared/src/main/scala/cats/parse/RadixNode.scala
index 082689d0..7b6f580f 100644
--- a/core/shared/src/main/scala/cats/parse/RadixNode.scala
+++ b/core/shared/src/main/scala/cats/parse/RadixNode.scala
@@ -28,11 +28,11 @@ import scala.annotation.tailrec
 import cats.syntax.all._
 
 private[parse] final class RadixNode(
-    protected val matched: String,
-    protected val bitMask: Int,
+    matched: String,
+    bitMask: Int,
     // the prefixes are the rest of the string after the fsts (not including the fsts Char)
-    protected val prefixes: Array[String],
-    protected val children: Array[RadixNode]
+    prefixes: Array[String],
+    children: Array[RadixNode]
 ) {
   override def toString(): String = {
     def list[A](ary: Array[A]): String = ary.mkString("[", ", ", "]")
@@ -56,25 +56,33 @@ private[parse] final class RadixNode(
       case nonNull => nonNull.length
     }
 
-  @tailrec
   final def matchAtOrNull(str: String, offset: Int): String =
-    if (offset < str.length)  {
+    if (offset < 0) null
+    else matchAtOrNullLoop(str, offset)
+
+  // loop invariant: 0 <= offset
+  @tailrec
+  final protected def matchAtOrNullLoop(str: String, offset: Int): String =
+    if (offset < str.length) {
       val c = str.charAt(offset)
       // this is a hash of c
       val idx = c.toInt & bitMask
       val prefix = prefixes(idx)
       if (prefix ne null) {
         // accept the prefix fo this character
-        if (str.regionMatches(offset, prefix, 0, prefix.length)) {
-          children(idx).matchAtOrNull(str, offset + prefix.length)
+        val plen = prefix.length
+        if (str.regionMatches(offset, prefix, 0, plen)) {
+          children(idx).matchAtOrNullLoop(str, offset + plen)
         } else {
           matched
         }
       } else {
         matched
       }
-    }
-    else {
+    } else if (offset > str.length) {
+      null
+    } else {
+      // this is only the case where offset == str.length
       matched
     }
 }
@@ -83,56 +91,55 @@ private[parse] object RadixNode {
   private val emptyStringArray = new Array[String](1)
   private val emptyChildrenArray = new Array[RadixNode](1)
 
-  private def fromNonEmpty(prefix: String, nonEmpty: NonEmptyList[String]): RadixNode = {
-    val nonEmpties = nonEmpty.toList.filter(_.nonEmpty)
+  private def fromTree(prefix: String, rest: List[String]): RadixNode = {
+    val nonEmpties = rest.toList.filter(_.nonEmpty)
     val headKeys = nonEmpties.iterator.map(_.head).toSet
 
-    @tailrec
-    def findBitMask(b: Int): Int =
-      if (b == 0xffff) b // biggest it can be
-      else {
-        val allDistinct = headKeys.iterator.map { c => c.toInt & b }.toSet.size == headKeys.size
-        if (allDistinct) b
-        else findBitMask((b << 1) | 1)
+    // If nonEmpty contains the empty string, we have a valid prefix
+    val thisPrefix = if (rest.exists(_.isEmpty)) prefix else null
+
+    if (nonEmpties.isEmpty) {
+      new RadixNode(thisPrefix, 0, emptyStringArray, emptyChildrenArray)
+    } else {
+      @tailrec
+      def findBitMask(b: Int): Int =
+        if (b == 0xffff) b // biggest it can be
+        else {
+          val allDistinct = headKeys.iterator.map { c => c.toInt & b }.toSet.size == headKeys.size
+          if (allDistinct) b
+          else findBitMask((b << 1) | 1)
+        }
+
+      val bitMask = findBitMask(0)
+      val branching = bitMask + 1
+      val prefixes = new Array[String](branching)
+      val children = new Array[RadixNode](branching)
+      val tree = nonEmpties.groupByNel { s => (s.head.toInt & bitMask) }
+      tree.foreach { case (idx, strings) =>
+        // strings is a NonEmptyList[String] which all start with the same char
+        val prefix1 = strings.reduce(commonPrefixSemilattice)
+        val plen = prefix1.length
+        val node = fromTree(prefix + prefix1, strings.map(_.drop(plen)).toList)
+        prefixes(idx) = prefix1
+        children(idx) = node
       }
 
-    val bitMask = findBitMask((1 << Integer.highestOneBit(headKeys.size)) - 1)
-    val branching = bitMask + 1
-    val prefixes = new Array[String](branching)
-    val children = new Array[RadixNode](branching)
-    val tree = nonEmpties.groupByNel { s => (s.head.toInt & bitMask) }
-    tree.foreach { case (idx, strings) =>
-      // strings is a NonEmptyList[String] which all start with the same char
-      val prefix1 = strings.reduce(commonPrefixSemilattice)
-      val plen = prefix1.length
-      val node = fromNonEmpty(prefix + prefix1, strings.map(_.drop(plen)))
-      prefixes(idx) = prefix1
-      children(idx) = node
+      new RadixNode(thisPrefix, bitMask, prefixes, children)
     }
-
-    // If nonEmpty contains the empty string, we have a valid prefix
-    val thisPrefix = if (nonEmpty.exists(_.isEmpty)) prefix else null
-    new RadixNode(thisPrefix, bitMask, prefixes, children)
   }
 
   def fromSortedStrings(strings: NonEmptyList[String]): RadixNode =
-    fromNonEmpty("", strings)
+    fromTree("", strings.distinct.toList)
 
   def fromStrings(strs: Iterable[String]): RadixNode =
-    NonEmptyList.fromList(strs.toList.distinct) match {
-      case Some(nel) => fromSortedStrings(nel)
-      case None => empty
-    }
-
-  private val empty = new RadixNode(null, 0, emptyStringArray, emptyChildrenArray)
+    fromTree("", strs.toList.distinct)
 
   final def commonPrefixLength(s1: String, s2: String): Int = {
     val len = Integer.min(s1.length, s2.length)
     var idx = 0
-    var cont = true
-    while (cont) {
-      if ((idx >= len) || (s1.charAt(idx) != s2.charAt(idx))) {
-        cont = false
+    while (idx < len) {
+      if (s1.charAt(idx) != s2.charAt(idx)) {
+        return idx
       } else {
         idx = idx + 1
       }
diff --git a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
index df594f3b..0b870102 100644
--- a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
+++ b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
@@ -21,7 +21,7 @@
 
 package cats.parse
 
-import org.scalacheck.Prop
+import org.scalacheck.{Gen, Prop}
 import org.scalacheck.Prop.forAll
 
 class RadixNodeTest extends munit.ScalaCheckSuite {
@@ -73,7 +73,11 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
       val ss = ss0.filter(_.nonEmpty)
       val radix = RadixNode.fromStrings(ss)
       val matchLen = radix.matchAt(target, 0)
-      assertEquals(matchLen >= 0, ss.exists(target.startsWith(_)), s"ss=$ss, ss.size = ${ss.size}, matchLen=$matchLen, radix=$radix")
+      assertEquals(
+        matchLen >= 0,
+        ss.exists(target.startsWith(_)),
+        s"ss=$ss, ss.size = ${ss.size}, matchLen=$matchLen, radix=$radix"
+      )
     }
 
     val p1 = forAll { (ss: List[String], head: Char, tail: String) =>
@@ -88,12 +92,6 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
     regressions.foldLeft(p1) { case (p, (ss, t)) => p && law(ss, t) }
   }
 
-  property("fromString(Nil) matches nothing") {
-    forAll { (s: String) =>
-      assert(RadixNode.fromStrings(Nil).matchAt(s, 0) < 0)
-    }
-  }
-
   property("we match everything in the set") {
     forAll { (ss0: List[String], head: Char, tail: String) =>
       val s1 = s"$head$tail"
@@ -122,4 +120,58 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
       assertEquals(left, right)
     }
   }
+
+  property("RadixNode.fromStrings(emptyString :: Nil) matches everything") {
+    val nilRadix = RadixNode.fromStrings("" :: Nil)
+    forAll { (targ: String) =>
+      forAll(Gen.choose(0, targ.length)) { off =>
+        assertEquals(nilRadix.matchAtOrNull(targ, off), "")
+      }
+    }
+  }
+
+  property("fromString(Nil) matches nothing") {
+    forAll { (s: String) =>
+      forAll(Gen.choose(-1, s.length + 1)) { off =>
+        assert(RadixNode.fromStrings(Nil).matchAt(s, off) < 0)
+      }
+    }
+  }
+
+  property("RadixTree singleton") {
+    forAll { (s: String, prefix: String) =>
+      val tree = RadixNode.fromStrings(s :: Nil)
+      assertEquals(tree.matchAtOrNull(prefix + s, prefix.length), s)
+    }
+  }
+
+  property("RadixTree union property") {
+    forAll { (t1: List[String], t2: List[String], targ: String) =>
+      val tree1 = RadixNode.fromStrings(t1)
+      val tree2 = RadixNode.fromStrings(t2)
+      val tree3 = RadixNode.fromStrings(t1 ::: t2)
+
+      forAll(Gen.choose(-1, targ.length + 1)) { off =>
+        val m1 = math.max(tree1.matchAt(targ, off), tree2.matchAt(targ, off))
+        assertEquals(m1, tree3.matchAt(targ, off))
+      }
+    }
+  }
+
+  property("matchAtOrNull is consistent") {
+    forAll { (args: List[String], targ: String) =>
+      val radix = RadixNode.fromStrings(args)
+      forAll(Gen.choose(-1, targ.length + 1)) { off =>
+        radix.matchAt(targ, off) match {
+          case x if x < 0 =>
+            assertEquals(radix.matchAtOrNull(targ, off), null)
+          case len =>
+            val left = radix.matchAtOrNull(targ, off)
+            assert(off + len <= targ.length, s"len = $len, off = $off")
+            assertEquals(left, targ.substring(off, off + len), s"len = $len, left = $left")
+        }
+      }
+    }
+  }
+
 }

From daab41e5488d5e8e6dfd689785a2f4e90b5738ff Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Sat, 15 Jan 2022 15:02:11 -1000
Subject: [PATCH 04/11] fix a corner case

---
 core/shared/src/main/scala/cats/parse/RadixNode.scala  | 10 +++++-----
 .../src/test/scala/cats/parse/RadixNodeTest.scala      |  8 ++++++--
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/core/shared/src/main/scala/cats/parse/RadixNode.scala b/core/shared/src/main/scala/cats/parse/RadixNode.scala
index 7b6f580f..6dcf747d 100644
--- a/core/shared/src/main/scala/cats/parse/RadixNode.scala
+++ b/core/shared/src/main/scala/cats/parse/RadixNode.scala
@@ -91,12 +91,12 @@ private[parse] object RadixNode {
   private val emptyStringArray = new Array[String](1)
   private val emptyChildrenArray = new Array[RadixNode](1)
 
-  private def fromTree(prefix: String, rest: List[String]): RadixNode = {
+  private def fromTree(prevMatch: String, prefix: String, rest: List[String]): RadixNode = {
     val nonEmpties = rest.toList.filter(_.nonEmpty)
     val headKeys = nonEmpties.iterator.map(_.head).toSet
 
     // If nonEmpty contains the empty string, we have a valid prefix
-    val thisPrefix = if (rest.exists(_.isEmpty)) prefix else null
+    val thisPrefix = if (rest.exists(_.isEmpty)) prefix else prevMatch
 
     if (nonEmpties.isEmpty) {
       new RadixNode(thisPrefix, 0, emptyStringArray, emptyChildrenArray)
@@ -119,7 +119,7 @@ private[parse] object RadixNode {
         // strings is a NonEmptyList[String] which all start with the same char
         val prefix1 = strings.reduce(commonPrefixSemilattice)
         val plen = prefix1.length
-        val node = fromTree(prefix + prefix1, strings.map(_.drop(plen)).toList)
+        val node = fromTree(thisPrefix, prefix + prefix1, strings.map(_.drop(plen)).toList)
         prefixes(idx) = prefix1
         children(idx) = node
       }
@@ -129,10 +129,10 @@ private[parse] object RadixNode {
   }
 
   def fromSortedStrings(strings: NonEmptyList[String]): RadixNode =
-    fromTree("", strings.distinct.toList)
+    fromTree(null, "", strings.distinct.toList)
 
   def fromStrings(strs: Iterable[String]): RadixNode =
-    fromTree("", strs.toList.distinct)
+    fromTree(null, "", strs.toList.distinct)
 
   final def commonPrefixLength(s1: String, s2: String): Int = {
     val len = Integer.min(s1.length, s2.length)
diff --git a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
index 0b870102..833d6c63 100644
--- a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
+++ b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
@@ -139,9 +139,9 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
   }
 
   property("RadixTree singleton") {
-    forAll { (s: String, prefix: String) =>
+    forAll { (s: String, prefix: String, suffix: String) =>
       val tree = RadixNode.fromStrings(s :: Nil)
-      assertEquals(tree.matchAtOrNull(prefix + s, prefix.length), s)
+      assertEquals(tree.matchAtOrNull(prefix + s + suffix, prefix.length), s)
     }
   }
 
@@ -174,4 +174,8 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
     }
   }
 
+  test("example from ParserTest") {
+    val tree = RadixNode.fromStrings(List("foo", "foobar", "foofoo", "foobat"))
+    assertEquals(tree.matchAtOrNull("foobal", 0), "foo")
+  }
 }

From 58fa16f00d7ce56a1836ad70687f50b27b8017fd Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Sat, 15 Jan 2022 15:04:44 -1000
Subject: [PATCH 05/11] fix 2.11 compilation

---
 core/shared/src/main/scala/cats/parse/RadixNode.scala | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/core/shared/src/main/scala/cats/parse/RadixNode.scala b/core/shared/src/main/scala/cats/parse/RadixNode.scala
index 6dcf747d..e5e9f9a8 100644
--- a/core/shared/src/main/scala/cats/parse/RadixNode.scala
+++ b/core/shared/src/main/scala/cats/parse/RadixNode.scala
@@ -25,8 +25,6 @@ import cats.data.NonEmptyList
 import cats.kernel.Semilattice
 import scala.annotation.tailrec
 
-import cats.syntax.all._
-
 private[parse] final class RadixNode(
     matched: String,
     bitMask: Int,
@@ -114,10 +112,10 @@ private[parse] object RadixNode {
       val branching = bitMask + 1
       val prefixes = new Array[String](branching)
       val children = new Array[RadixNode](branching)
-      val tree = nonEmpties.groupByNel { s => (s.head.toInt & bitMask) }
+      val tree = nonEmpties.groupBy { s => (s.head.toInt & bitMask) }
       tree.foreach { case (idx, strings) =>
         // strings is a NonEmptyList[String] which all start with the same char
-        val prefix1 = strings.reduce(commonPrefixSemilattice)
+        val prefix1 = strings.reduce(commonPrefixSemilattice.combine(_, _))
         val plen = prefix1.length
         val node = fromTree(thisPrefix, prefix + prefix1, strings.map(_.drop(plen)).toList)
         prefixes(idx) = prefix1
@@ -129,7 +127,7 @@ private[parse] object RadixNode {
   }
 
   def fromSortedStrings(strings: NonEmptyList[String]): RadixNode =
-    fromTree(null, "", strings.distinct.toList)
+    fromTree(null, "", strings.toList.distinct)
 
   def fromStrings(strs: Iterable[String]): RadixNode =
     fromTree(null, "", strs.toList.distinct)

From eca6ade5b14b2adbf33143281facaa20b6b7e832 Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Mon, 17 Jan 2022 09:32:33 -1000
Subject: [PATCH 06/11] Add some new benchmarks to exercise radixnode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

results:

[info] Benchmark                         (test)  Mode  Cnt      Score      Error  Units
[info] StringInBenchmarks.linearMatchIn     foo  avgt    3     77.718 ±    3.173  ns/op
[info] StringInBenchmarks.linearMatchIn   broad  avgt    3  92973.234 ± 2156.151  ns/op
[info] StringInBenchmarks.oneOfParse        foo  avgt    3     91.240 ±    4.854  ns/op
[info] StringInBenchmarks.oneOfParse      broad  avgt    3    806.046 ±   37.539  ns/op
[info] StringInBenchmarks.radixMatchIn      foo  avgt    3     59.750 ±    1.315  ns/op
[info] StringInBenchmarks.radixMatchIn    broad  avgt    3    769.000 ±   25.609  ns/op
[info] StringInBenchmarks.stringInParse     foo  avgt    3     91.551 ±    0.600  ns/op
[info] StringInBenchmarks.stringInParse   broad  avgt    3    809.561 ±   54.042  ns/op
---
 .../cats/parse/bench/StringInBench.scala      | 40 ++++++++++++++-----
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/bench/src/main/scala/cats/parse/bench/StringInBench.scala b/bench/src/main/scala/cats/parse/bench/StringInBench.scala
index 924f06a9..af98a76b 100644
--- a/bench/src/main/scala/cats/parse/bench/StringInBench.scala
+++ b/bench/src/main/scala/cats/parse/bench/StringInBench.scala
@@ -30,20 +30,40 @@ import org.openjdk.jmh.annotations._
 @BenchmarkMode(Array(Mode.AverageTime))
 @OutputTimeUnit(TimeUnit.NANOSECONDS)
 class StringInBenchmarks {
-  val inputs =
-    List("foofoo", "bar", "foobat", "foot", "foobar")
+  @Param(Array("foo", "broad"))
+  var test: String = _
 
-  val stringsToMatch =
-    "foobar" :: "foofoo" :: "foobaz" :: "foo" :: "bar" :: Nil
+  var inputs: List[String] = _
 
-  val radixNode = RadixNode.fromStrings(stringsToMatch)
+  var stringsToMatch: List[String] = _
 
-  val stringIn = Parser.stringIn(stringsToMatch)
+  var radixNode: RadixNode = _
 
-  val oneOf =
-    Parser.oneOf(
-      stringsToMatch.map { s => Parser.string(s) }
-    )
+  var stringIn: Parser[Unit] = _
+
+  var oneOf: Parser[Unit] = _
+
+  @Setup(Level.Trial)
+  def setup(): Unit = {
+    if (test == "foo") {
+      inputs = List("foofoo", "bar", "foobat", "foot", "foobar")
+      stringsToMatch = List("foobar", "foofoo", "foobaz", "foo", "bar")
+    }
+    else if (test == "broad") {
+      // test all lower ascii strings like aaaa, aaab, aaac, ... bbba, bbbb, bbbc, ...
+      stringsToMatch = (for {
+        h <- 'a' to 'z'
+        t <- 'a' to 'z'
+      } yield s"$h$h$h$t").toList
+      
+      // take 10% of the inputs
+      inputs = stringsToMatch.filter(_.hashCode % 10 == 0)
+    }
+
+    radixNode = RadixNode.fromStrings(stringsToMatch)
+    stringIn = Parser.stringIn(stringsToMatch).void
+    oneOf = Parser.oneOf(stringsToMatch.map(Parser.string(_)))
+  }
 
   @Benchmark
   def stringInParse(): Unit =

From d1747031942fe9d66f35e10fc920665df0c80c4d Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Mon, 17 Jan 2022 10:12:56 -1000
Subject: [PATCH 07/11] run format

---
 bench/src/main/scala/cats/parse/bench/StringInBench.scala | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/bench/src/main/scala/cats/parse/bench/StringInBench.scala b/bench/src/main/scala/cats/parse/bench/StringInBench.scala
index af98a76b..42330cad 100644
--- a/bench/src/main/scala/cats/parse/bench/StringInBench.scala
+++ b/bench/src/main/scala/cats/parse/bench/StringInBench.scala
@@ -48,14 +48,13 @@ class StringInBenchmarks {
     if (test == "foo") {
       inputs = List("foofoo", "bar", "foobat", "foot", "foobar")
       stringsToMatch = List("foobar", "foofoo", "foobaz", "foo", "bar")
-    }
-    else if (test == "broad") {
+    } else if (test == "broad") {
       // test all lower ascii strings like aaaa, aaab, aaac, ... bbba, bbbb, bbbc, ...
       stringsToMatch = (for {
         h <- 'a' to 'z'
         t <- 'a' to 'z'
       } yield s"$h$h$h$t").toList
-      
+
       // take 10% of the inputs
       inputs = stringsToMatch.filter(_.hashCode % 10 == 0)
     }

From b066967ea15fc11b15dbd8d84139fd3ce0367676 Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Mon, 17 Jan 2022 10:21:25 -1000
Subject: [PATCH 08/11] fix warning on StringInBenchmarks

---
 bench/src/main/scala/cats/parse/bench/StringInBench.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bench/src/main/scala/cats/parse/bench/StringInBench.scala b/bench/src/main/scala/cats/parse/bench/StringInBench.scala
index 42330cad..f4307a26 100644
--- a/bench/src/main/scala/cats/parse/bench/StringInBench.scala
+++ b/bench/src/main/scala/cats/parse/bench/StringInBench.scala
@@ -29,7 +29,7 @@ import org.openjdk.jmh.annotations._
 @State(Scope.Benchmark)
 @BenchmarkMode(Array(Mode.AverageTime))
 @OutputTimeUnit(TimeUnit.NANOSECONDS)
-class StringInBenchmarks {
+private[parse] class StringInBenchmarks {
   @Param(Array("foo", "broad"))
   var test: String = _
 

From 05d274e223747be6afc30264ca41efd0a832883f Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Mon, 17 Jan 2022 10:58:54 -1000
Subject: [PATCH 09/11] add another test, cleanup

---
 .../src/main/scala/cats/parse/RadixNode.scala | 25 ++++++++++++++++---
 .../test/scala/cats/parse/RadixNodeTest.scala |  6 +++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/core/shared/src/main/scala/cats/parse/RadixNode.scala b/core/shared/src/main/scala/cats/parse/RadixNode.scala
index e5e9f9a8..669e2fa4 100644
--- a/core/shared/src/main/scala/cats/parse/RadixNode.scala
+++ b/core/shared/src/main/scala/cats/parse/RadixNode.scala
@@ -39,6 +39,22 @@ private[parse] final class RadixNode(
     s"RadixNode($matched, $bitMask, $ps, $cs)"
   }
 
+  /** @return
+    *   all strings that are in this RadixNode
+    */
+  def allStrings: List[String] = {
+    // matched may be null (meaning there is not yet a partial match)
+    // or it may be non-null: there is a partial match
+    // if it is non-null it may be duplicated in children
+    val rest = children.iterator.flatMap {
+      case null => Nil
+      case c => c.allStrings
+    }
+
+    if (matched eq null) rest.toList
+    else (matched :: rest.filterNot(_ == matched).toList)
+  }
+
   /** If this matches, return the new offset, else return -1
     *
     * @param str
@@ -90,8 +106,7 @@ private[parse] object RadixNode {
   private val emptyChildrenArray = new Array[RadixNode](1)
 
   private def fromTree(prevMatch: String, prefix: String, rest: List[String]): RadixNode = {
-    val nonEmpties = rest.toList.filter(_.nonEmpty)
-    val headKeys = nonEmpties.iterator.map(_.head).toSet
+    val nonEmpties = rest.filter(_.nonEmpty)
 
     // If nonEmpty contains the empty string, we have a valid prefix
     val thisPrefix = if (rest.exists(_.isEmpty)) prefix else prevMatch
@@ -99,6 +114,7 @@ private[parse] object RadixNode {
     if (nonEmpties.isEmpty) {
       new RadixNode(thisPrefix, 0, emptyStringArray, emptyChildrenArray)
     } else {
+      val headKeys = nonEmpties.iterator.map(_.head).toSet
       @tailrec
       def findBitMask(b: Int): Int =
         if (b == 0xffff) b // biggest it can be
@@ -117,7 +133,8 @@ private[parse] object RadixNode {
         // strings is a NonEmptyList[String] which all start with the same char
         val prefix1 = strings.reduce(commonPrefixSemilattice.combine(_, _))
         val plen = prefix1.length
-        val node = fromTree(thisPrefix, prefix + prefix1, strings.map(_.drop(plen)).toList)
+        val s1 = if (plen == 0) strings else strings.map(_.drop(plen))
+        val node = fromTree(thisPrefix, prefix + prefix1, s1)
         prefixes(idx) = prefix1
         children(idx) = node
       }
@@ -126,6 +143,8 @@ private[parse] object RadixNode {
     }
   }
 
+  /** This is identical to fromStrings and only here for binary compatibility
+    */
   def fromSortedStrings(strings: NonEmptyList[String]): RadixNode =
     fromTree(null, "", strings.toList.distinct)
 
diff --git a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
index 833d6c63..b695643f 100644
--- a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
+++ b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
@@ -178,4 +178,10 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
     val tree = RadixNode.fromStrings(List("foo", "foobar", "foofoo", "foobat"))
     assertEquals(tree.matchAtOrNull("foobal", 0), "foo")
   }
+
+  property("RadixNode.allStrings roundTrips") {
+    forAll { (ss: List[String]) =>
+      assertEquals(RadixNode.fromStrings(ss).allStrings.sorted, ss.distinct.sorted)
+    }
+  }
 }

From 62c7da8c06f00790aaf1ffd7ccf7c254b1e420c2 Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Mon, 17 Jan 2022 12:56:34 -1000
Subject: [PATCH 10/11] code polish

---
 .../src/main/scala/cats/parse/RadixNode.scala | 51 ++++++++++++-------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/core/shared/src/main/scala/cats/parse/RadixNode.scala b/core/shared/src/main/scala/cats/parse/RadixNode.scala
index 669e2fa4..7002b64f 100644
--- a/core/shared/src/main/scala/cats/parse/RadixNode.scala
+++ b/core/shared/src/main/scala/cats/parse/RadixNode.scala
@@ -71,10 +71,10 @@ private[parse] final class RadixNode(
     }
 
   final def matchAtOrNull(str: String, offset: Int): String =
-    if (offset < 0) null
+    if ((offset < 0) || (str.length < offset)) null
     else matchAtOrNullLoop(str, offset)
 
-  // loop invariant: 0 <= offset
+  // loop invariant: 0 <= offset <= str.length
   @tailrec
   final protected def matchAtOrNullLoop(str: String, offset: Int): String =
     if (offset < str.length) {
@@ -83,7 +83,12 @@ private[parse] final class RadixNode(
       val idx = c.toInt & bitMask
       val prefix = prefixes(idx)
       if (prefix ne null) {
-        // accept the prefix fo this character
+        /*
+         * this prefix *may* match here, but may not
+         * note we only know that c & bitMask matches
+         * what the prefix has to be, it could differ
+         * on other bits.
+         */
         val plen = prefix.length
         if (str.regionMatches(offset, prefix, 0, plen)) {
           children(idx).matchAtOrNullLoop(str, offset + plen)
@@ -93,10 +98,9 @@ private[parse] final class RadixNode(
       } else {
         matched
       }
-    } else if (offset > str.length) {
-      null
     } else {
       // this is only the case where offset == str.length
+      // due to our invariant
       matched
     }
 }
@@ -106,20 +110,29 @@ private[parse] object RadixNode {
   private val emptyChildrenArray = new Array[RadixNode](1)
 
   private def fromTree(prevMatch: String, prefix: String, rest: List[String]): RadixNode = {
-    val nonEmpties = rest.filter(_.nonEmpty)
+    val (nonEmpties, empties) = rest.partition(_.nonEmpty)
 
-    // If nonEmpty contains the empty string, we have a valid prefix
-    val thisPrefix = if (rest.exists(_.isEmpty)) prefix else prevMatch
+    // If rest contains the empty string, we have a valid prefix
+    val thisPrefix = if (empties.nonEmpty) prefix else prevMatch
 
     if (nonEmpties.isEmpty) {
       new RadixNode(thisPrefix, 0, emptyStringArray, emptyChildrenArray)
     } else {
       val headKeys = nonEmpties.iterator.map(_.head).toSet
+      /*
+       * The idea here is to use b lowest bits of the char
+       * as an index into the array, with the smallest
+       * number b such that all the keys are unique & b
+       */
       @tailrec
       def findBitMask(b: Int): Int =
         if (b == 0xffff) b // biggest it can be
         else {
-          val allDistinct = headKeys.iterator.map { c => c.toInt & b }.toSet.size == headKeys.size
+          val hs = headKeys.size
+          val allDistinct =
+            // they can't all be distinct if the size isn't as big as the headKeys size
+            ((b + 1) >= hs) &&
+              (headKeys.iterator.map { c => c.toInt & b }.toSet.size == hs)
           if (allDistinct) b
           else findBitMask((b << 1) | 1)
         }
@@ -128,16 +141,16 @@ private[parse] object RadixNode {
       val branching = bitMask + 1
       val prefixes = new Array[String](branching)
       val children = new Array[RadixNode](branching)
-      val tree = nonEmpties.groupBy { s => (s.head.toInt & bitMask) }
-      tree.foreach { case (idx, strings) =>
-        // strings is a NonEmptyList[String] which all start with the same char
-        val prefix1 = strings.reduce(commonPrefixSemilattice.combine(_, _))
-        val plen = prefix1.length
-        val s1 = if (plen == 0) strings else strings.map(_.drop(plen))
-        val node = fromTree(thisPrefix, prefix + prefix1, s1)
-        prefixes(idx) = prefix1
-        children(idx) = node
-      }
+      nonEmpties
+        .groupBy { s => (s.head.toInt & bitMask) }
+        .foreach { case (idx, strings) =>
+          // strings is a non-empty List[String] which all start with the same char
+          val prefix1 = strings.reduce(commonPrefixSemilattice.combine(_, _))
+          // note prefix1.length >= 1 because they all match on the first character
+          prefixes(idx) = prefix1
+          children(idx) =
+            fromTree(thisPrefix, prefix + prefix1, strings.map(_.drop(prefix1.length)))
+        }
 
       new RadixNode(thisPrefix, bitMask, prefixes, children)
     }

From d79bcfa51fa51f6e7361d28507ce053690b69e66 Mon Sep 17 00:00:00 2001
From: Patrick Oscar Boykin <boykin@pobox.com>
Date: Mon, 17 Jan 2022 13:07:29 -1000
Subject: [PATCH 11/11] one more test...

---
 core/shared/src/test/scala/cats/parse/RadixNodeTest.scala | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
index b695643f..991294b3 100644
--- a/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
+++ b/core/shared/src/test/scala/cats/parse/RadixNodeTest.scala
@@ -121,6 +121,14 @@ class RadixNodeTest extends munit.ScalaCheckSuite {
     }
   }
 
+  property("commonPrefix is finds prefix") {
+    val sl = RadixNode.commonPrefixSemilattice
+    forAll { (s0: String, suffix: String) =>
+      assertEquals(sl.combine(s0, s0 + suffix), s0)
+      assertEquals(sl.combine(s0 + suffix, s0), s0)
+    }
+  }
+
   property("RadixNode.fromStrings(emptyString :: Nil) matches everything") {
     val nilRadix = RadixNode.fromStrings("" :: Nil)
     forAll { (targ: String) =>