ir: store FileInfo string in escaped format (#1690)

This should speed up the common case
as the compiler never operates on the
unescaped string.
The new escape function also fixes a bug
where ']' was not escaped even though it
is the delimiting character for FileInfo.
In order to maintain backwards
compatibility for the ProtoBuf format,
this patch adds escape/unescape calls
when going from/to protobuf format.
For better performance we should consider
changing the protobuf format.

4 files changed, 70 insertions, 0 deletions

diff --git a/src/test/scala/firrtlTests/InfoSpec.scala b/src/test/scala/firrtlTests/InfoSpec.scala
index 48567d69..01e0a0ac 100644
--- a/src/test/scala/firrtlTests/InfoSpec.scala
+++ b/src/test/scala/firrtlTests/InfoSpec.scala
@@ -172,4 +172,27 @@ class InfoSpec extends FirrtlFlatSpec with FirrtlMatchers {
     val expectedInfos = Seq(FileInfo(StringLit("Top.scala 15:14")), FileInfo(StringLit("myfile.fir 6:4")))
     circuitState should containTree { case MultiInfo(`expectedInfos`) => true }
   }
+
+  "FileInfo" should "be able to contain a escaped characters" in {
+    def input(info: String): String =
+      s"""circuit m: @[$info]
+        |  module m:
+        |    skip
+        |""".stripMargin
+    def parseInfo(info: String): FileInfo = {
+      firrtl.Parser.parse(input(info)).info.asInstanceOf[FileInfo]
+    }
+
+    parseInfo("test\\ntest").escaped should be ("test\\ntest")
+    parseInfo("test\\ntest").unescaped should be ("test\ntest")
+    parseInfo("test\\ttest").escaped should be ("test\\ttest")
+    parseInfo("test\\ttest").unescaped should be ("test\ttest")
+    parseInfo("test\\\\test").escaped should be ("test\\\\test")
+    parseInfo("test\\\\test").unescaped should be ("test\\test")
+    parseInfo("test\\]test").escaped should be ("test\\]test")
+    parseInfo("test\\]test").unescaped should be ("test]test")
+    parseInfo("test[\\][\\]test").escaped should be ("test[\\][\\]test")
+    parseInfo("test[\\][\\]test").unescaped should be ("test[][]test")
+  }
+
 }
diff --git a/src/test/scala/firrtlTests/ParserSpec.scala b/src/test/scala/firrtlTests/ParserSpec.scala
index 2ae5b430..ba5cb889 100644
--- a/src/test/scala/firrtlTests/ParserSpec.scala
+++ b/src/test/scala/firrtlTests/ParserSpec.scala
@@ -244,6 +244,18 @@ class ParserSpec extends FirrtlFlatSpec {
       Driver.execute(manager)
     }
   }
+
+  it should "be able to parse a MultiInfo as a FileInfo" in {
+    // currently MultiInfo gets flattened into a single string which can only be recovered as a FileInfo
+    val info = ir.MultiInfo(Seq(ir.MultiInfo(Seq(ir.FileInfo("a"))), ir.FileInfo("b"), ir.FileInfo("c")))
+    val input =
+      s"""circuit m:${info.serialize}
+        |  module m:
+        |    skip
+        |""".stripMargin
+    val c = firrtl.Parser.parse(input)
+    assert(c.info == ir.FileInfo("a b c"))
+  }
 }
 
 class ParserPropSpec extends FirrtlPropSpec {
diff --git a/src/test/scala/firrtlTests/ProtoBufSpec.scala b/src/test/scala/firrtlTests/ProtoBufSpec.scala
index 14f94cb3..3a94ec3f 100644
--- a/src/test/scala/firrtlTests/ProtoBufSpec.scala
+++ b/src/test/scala/firrtlTests/ProtoBufSpec.scala
@@ -211,4 +211,19 @@ class ProtoBufSpec extends FirrtlFlatSpec {
     val expected = ir.ValidIf(ir.Reference("en"), ir.Reference("x"), UnknownType)
     FromProto.convert(ToProto.convert(vi).build) should equal (expected)
   }
+
+  it should "appropriately escape and unescape FileInfo strings" in {
+    val pairs = Seq(
+      "test\\ntest" -> "test\ntest",
+      "test\\ttest" -> "test\ttest",
+      "test\\\\test" -> "test\\test",
+      "test\\]test" -> "test]test"
+    )
+
+    pairs.foreach { case (escaped, unescaped) =>
+      val info = ir.FileInfo(escaped)
+      ToProto.convert(info).build().getText should equal (unescaped)
+      FromProto.convert(ToProto.convert(info).build) should equal (info)
+    }
+  }
 }
diff --git a/src/test/scala/firrtlTests/VerilogEmitterTests.scala b/src/test/scala/firrtlTests/VerilogEmitterTests.scala
index 171bce78..21d7075e 100644
--- a/src/test/scala/firrtlTests/VerilogEmitterTests.scala
+++ b/src/test/scala/firrtlTests/VerilogEmitterTests.scala
@@ -727,6 +727,26 @@ class VerilogEmitterSpec extends FirrtlFlatSpec {
     result should    containLine("wire [2:0] _GEN_0 = $signed(x) - 3'sh2;")
     result should    containLine("assign z = _GEN_0[1:0];")
   }
+
+  it should "emit FileInfo as Verilog comment" in {
+    def result(info: String): CircuitState = compileBody(
+      s"""input x : UInt<2>
+        |output z : UInt<2>
+        |z <= x @[$info]
+        |""".stripMargin
+    )
+    result("test") should containLine("  assign z = x; // @[test]")
+    // newlines currently are supposed to be escaped for both firrtl and Verilog
+    // (alternatively one could emit a multi-line comment)
+    result("test\\nx") should containLine("  assign z = x; // @[test\\nx]")
+    // not sure why, but we are also escaping tabs
+    result("test\\tx") should containLine("  assign z = x; // @[test\\tx]")
+    // escaping closing square brackets is only a firrtl issue, should not be reflected in the Verilog emission
+    result("test\\]") should containLine("  assign z = x; // @[test]]")
+    // while firrtl allows for Unicode in the info field they should be escaped for Verilog
+    result("test \uD83D\uDE0E") should containLine("  assign z = x; // @[test \\uD83D\\uDE0E]")
+
+  }
 }
 
 class VerilogDescriptionEmitterSpec extends FirrtlFlatSpec {