Fix string lit (#663)

Refactor StringLit to use String instead of Array[Byte]

1 files changed, 46 insertions, 37 deletions

diff --git a/src/test/scala/firrtlTests/StringSpec.scala b/src/test/scala/firrtlTests/StringSpec.scala
index 7e7c040e..87ee9191 100644
--- a/src/test/scala/firrtlTests/StringSpec.scala
+++ b/src/test/scala/firrtlTests/StringSpec.scala
@@ -3,14 +3,17 @@
 package firrtlTests
 
 import firrtl._
+import firrtl.ir.StringLit
 
 import java.io._
 
 import scala.sys.process._
+import annotation.tailrec
 import org.scalatest._
 import org.scalatest.prop._
 import org.scalatest.Assertions._
 import org.scalacheck._
+import org.scalacheck.Arbitrary._
 
 class PrintfSpec extends FirrtlPropSpec {
 
@@ -55,17 +58,12 @@ class PrintfSpec extends FirrtlPropSpec {
 class StringSpec extends FirrtlPropSpec {
 
   // Whitelist is [0x20 - 0x7e]
-  val whitelist = 
-    """ !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ""" +
+  val whitelist =
+    """ !\"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ""" +
     """[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~"""
-  val whitelistBA: Array[Byte] = Array.range(0x20, 0x7e) map (_.toByte)
 
   property(s"Character whitelist should be supported: [$whitelist] ") {
-    val lit = firrtl.FIRRTLStringLitHandler.unescape(whitelist)
-    // Check internal
-    lit.array zip whitelistBA foreach { case (b, e) =>
-      assert(b == e, s"(${b.toChar} did not equal expected ${e.toChar})")
-    }
+    val lit = StringLit.unescape(whitelist)
     // Check result
     assert(lit.serialize == whitelist)
   }
@@ -74,40 +72,51 @@ class StringSpec extends FirrtlPropSpec {
   val esc = """\\\'\"\t\n"""
   val validEsc = Seq('n', 't', '\\', '"', '\'')
   property(s"Escape characters [$esc] should parse") {
-    val lit = firrtl.FIRRTLStringLitHandler.unescape(esc)
-    assert(lit.array(0) == 0x5c) 
-    assert(lit.array(1) == 0x27)
-    assert(lit.array(2) == 0x22)
-    assert(lit.array(3) == 0x09)
-    assert(lit.array(4) == 0x0a)
-    assert(lit.array.length == 5)
+    val lit = StringLit.unescape(esc)
+    assert(lit.string(0).toByte == 0x5c)
+    assert(lit.string(1).toByte == 0x27)
+    assert(lit.string(2).toByte == 0x22)
+    assert(lit.string(3).toByte == 0x09)
+    assert(lit.string(4).toByte == 0x0a)
+    assert(lit.string.length == 5)
   }
 
-  // Generators for random testing
-  val validChar = Gen.oneOf((0x20.toChar to 0x5b.toChar).toSeq ++
-                             (0x5e.toChar to 0x7e.toChar).toSeq) // exclude '\\'
-  val validCharSeq = Gen.containerOf[Seq, Char](validChar)
-  val invalidChar = Gen.oneOf(Gen.choose(0x00.toChar, 0x1f.toChar),
-                             Gen.choose(0x7f.toChar, 0xff.toChar))
-  val invalidEsc = Gen.oneOf((0x00.toChar to 0xff.toChar).toSeq diff validEsc)
-
-  property("Random invalid strings should fail") {
-    forAll(validCharSeq, invalidChar, validCharSeq) { 
-      (head: Seq[Char], bad: Char, tail: Seq[Char]) =>
-        val str = ((head :+ bad) ++ tail).mkString
-        intercept[InvalidStringLitException] {
-          firrtl.FIRRTLStringLitHandler.unescape(str)
+  // From IEEE 1364-2001 2.6
+  def isValidVerilogString(str: String): Boolean = {
+    @tailrec def rec(xs: List[Char]): Boolean = xs match {
+      case Nil => true
+      case '\\' :: esc =>
+        if (Set('n', 't', '\\', '"').contains(esc.head)) rec(esc.tail)
+        else { // Check valid octal escape, otherwise illegal
+          val next3 = esc.take(3)
+          if (next3.size == 3 && next3.forall(('0' to '7').toSet.contains)) rec(esc.drop(3))
+          else false
         }
+      case char :: tail => // Check Legal ASCII
+        if (char.toInt < 256 && char.toInt >= 0) rec(tail)
+        else false
     }
+    rec(str.toList)
   }
-    
-  property(s"Invalid escape characters should fail") {
-    forAll(validCharSeq, invalidEsc, validCharSeq) {
-      (head: Seq[Char], badEsc: Char, tail: Seq[Char]) =>
-        val str = (head ++ Seq('\\', badEsc) ++ tail).mkString
-        intercept[InvalidEscapeCharException] {
-          firrtl.FIRRTLStringLitHandler.unescape(str)
-        }
+  // From IEEE 1364-2001 17.1.1.2
+  val legalFormats = "HhDdOoBbCcLlVvMmSsTtUuZz%".toSet
+  def isValidVerilogFormat(str: String): Boolean = str.toSeq.sliding(2).forall {
+    case Seq('%', char) if legalFormats contains char => true
+    case _ => true
+  }
+
+  // Generators for legal Firrtl format strings
+  val genFormat = Gen.oneOf("bdxc%").map(List('%', _))
+  val genEsc = Gen.oneOf(esc.toSeq).map(List(_))
+  val genChar = arbitrary[Char].suchThat(c => (c != '%' && c != '\\')).map(List(_))
+  val genFragment = Gen.frequency((10, genChar), (1, genFormat), (1, genEsc)).map(_.mkString)
+  val genString = Gen.listOf[String](genFragment).map(_.mkString)
+
+  property ("Firrtl Format Strings with Unicode chars should emit as legal Verilog Strings") {
+    forAll (genString) { str =>
+      val verilogStr = StringLit(str).verilogFormat.verilogEscape
+      assert(isValidVerilogString(verilogStr))
+      assert(isValidVerilogFormat(verilogStr))
     }
   }
 }