Fix Printf macro to catch s-interpolator usages in Scala 2.13 (#3143) (…

…#3157) Co-authored-by: Jack Koenig <[email protected]> (cherry picked from commit f3d0f22) Co-authored-by: Aditya Naik <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
chipsalliance · Apr 4, 2023 · 39327df · 39327df
1 parent dc54a98
commit 39327df
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 41 deletions.
diff --git a/core/src/main/scala/chisel3/Printf.scala b/core/src/main/scala/chisel3/Printf.scala
@@ -36,6 +36,38 @@ object printf {
     formatIn.map(escaped).mkString("")
   }
 
+  private[chisel3] def _checkFormatString(c: blackbox.Context)(fmt: c.Tree): Unit = {
+    import c.universe._
+
+    val errorString = "The s-interpolator prints the Scala .toString of Data objects rather than the value " +
+      "of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
+      "an elaboration time print, use println."
+
+    // Error on Data in the AST by matching on the Scala 2.13 string
+    // interpolation lowering to concatenation
+    def throwOnChiselData(x: c.Tree): Unit = x match {
+      case q"$x+$y" => {
+        if (x.tpe <:< typeOf[chisel3.Data] || y.tpe <:< typeOf[chisel3.Data]) {
+          c.error(c.enclosingPosition, errorString)
+        } else {
+          throwOnChiselData(x)
+          throwOnChiselData(y)
+        }
+      }
+      case _ =>
+    }
+    throwOnChiselData(fmt)
+
+    fmt match {
+      case q"scala.StringContext.apply(..$_).s(..$_)" =>
+        c.error(
+          c.enclosingPosition,
+          errorString
+        )
+      case _ =>
+    }
+  }
+
   /** Named class for [[printf]]s. */
   final class Printf private[chisel3] (val pable: Printable) extends VerificationStatement
 
@@ -87,16 +119,7 @@ object printf {
     compileOptions: c.Tree
   ): c.Tree = {
     import c.universe._
-    fmt match {
-      case q"scala.StringContext.apply(..$_).s(..$_)" =>
-        c.error(
-          c.enclosingPosition,
-          "The s-interpolator prints the Scala .toString of Data objects rather than the value " +
-            "of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
-            "an elaboration time print, use println."
-        )
-      case _ =>
-    }
+    _checkFormatString(c)(fmt)
     val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("printfWithReset"))
     q"$apply_impl_do(_root_.chisel3.Printable.pack($fmt, ..$data))($sourceInfo, $compileOptions)"
   }

diff --git a/core/src/main/scala/chisel3/VerificationStatement.scala b/core/src/main/scala/chisel3/VerificationStatement.scala
@@ -92,16 +92,7 @@ object assert extends VerifPrintMacrosDoc {
     compileOptions: c.Tree
   ): c.Tree = {
     import c.universe._
-    message match {
-      case q"scala.StringContext.apply(..$_).s(..$_)" =>
-        c.error(
-          c.enclosingPosition,
-          "The s-interpolator prints the Scala .toString of Data objects rather than the value " +
-            "of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
-            "an elaboration time check, call assert with a Boolean condition."
-        )
-      case _ =>
-    }
+    printf._checkFormatString(c)(message)
     val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable"))
     q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some(_root_.chisel3.Printable.pack($message, ..$data)))($sourceInfo, $compileOptions)"
   }
@@ -273,16 +264,7 @@ object assume extends VerifPrintMacrosDoc {
     compileOptions: c.Tree
   ): c.Tree = {
     import c.universe._
-    message match {
-      case q"scala.StringContext.apply(..$_).s(..$_)" =>
-        c.error(
-          c.enclosingPosition,
-          "The s-interpolator prints the Scala .toString of Data objects rather than the value " +
-            "of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
-            "an elaboration time check, call assert with a Boolean condition."
-        )
-      case _ =>
-    }
+    printf._checkFormatString(c)(message)
     val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable"))
     q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some(_root_.chisel3.Printable.pack($message, ..$data)))($sourceInfo, $compileOptions)"
   }

diff --git a/docs/src/explanations/printing.md b/docs/src/explanations/printing.md
@@ -15,25 +15,16 @@ Chisel provides the `printf` function for debugging purposes. It comes in two fl
 
 Chisel also supports printf in a style similar to [Scala's String Interpolation](http://docs.scala-lang.org/overviews/core/string-interpolation.html). Chisel provides a custom string interpolator `cf` which follows C-style format specifiers (see section [C-style](#c-style) below).
 
-Note that the Scala s-interpolator is not supported in Chisel.
-Future versions of Chisel will throw an error when using the s-interpolator
-within a printf.
+Note that the Scala s-interpolator is not supported in Chisel constructs and will throw an error:
 
 ```scala mdoc:invisible
 import chisel3._
-
-//TODO: Make the following Module fail when using s-interpolator in a printf
-// This used to fail under 2.12 but the macro that implemented this does
-// not work properly in Scala 2.13
-// When fixed add mdoc:fail to following ```scala block and clean up
 ```
 
-```scala
+```scala mdoc:fail
 class MyModule extends Module {
   val in = IO(Input(UInt(8.W)))
   printf(s"in = $in\n")
-         ^
-         +----- Do NOT use the s-interpolator in a printf
 }
 ```