Warn on S-interpolator usage for assert, assume and printf (backport #2751) (#2757)

* Add internal methods to maintain binary compatibility

Co-authored-by: Megan Wachs <megan@sifive.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>

2 files changed, 101 insertions, 7 deletions

diff --git a/core/src/main/scala/chisel3/Printf.scala b/core/src/main/scala/chisel3/Printf.scala
index 9410a409..a7338072 100644
--- a/core/src/main/scala/chisel3/Printf.scala
+++ b/core/src/main/scala/chisel3/Printf.scala
@@ -2,10 +2,11 @@
 
 package chisel3
 
-import scala.language.experimental.macros
 import chisel3.internal._
 import chisel3.internal.Builder.pushCommand
 import chisel3.internal.sourceinfo.SourceInfo
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox
 
 /** Prints a message in simulation
   *
@@ -76,7 +77,44 @@ object printf {
     * @param data format string varargs containing data to print
     */
   def apply(fmt: String, data: Bits*)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf =
-    apply(Printable.pack(fmt, data: _*))
+    macro _applyMacroWithInterpolatorCheck
+
+  def _applyMacroWithInterpolatorCheck(
+    c:              blackbox.Context
+  )(fmt:            c.Tree,
+    data:           c.Tree*
+  )(sourceInfo:     c.Tree,
+    compileOptions: c.Tree
+  ): c.Tree = {
+    import c.universe._
+    fmt match {
+      case q"scala.StringContext.apply(..$_).s(..$_)" =>
+        c.warning(
+          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 _ =>
+    }
+    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)"
+  }
+
+  // Private internal methods that serve to maintain binary
+  // compatibility after interpolator check updates
+  @deprecated("This Printf.apply method has been deprecated and will be removed in Chisel 3.6")
+  def apply(fmt: String, sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf =
+    apply(fmt, Nil, sourceInfo, compileOptions)
+
+  @deprecated("This Printf.apply method has been deprecated and will be removed in Chisel 3.6")
+  def apply(
+    fmt:            String,
+    data:           Seq[Bits],
+    sourceInfo:     SourceInfo,
+    compileOptions: CompileOptions
+  ): Printf =
+    apply(Printable.pack(fmt, data: _*))(sourceInfo, compileOptions)
 
   /** Prints a message in simulation
     *
@@ -92,7 +130,15 @@ object printf {
     * @see [[Printable]] documentation
     * @param pable [[Printable]] to print
     */
-  def apply(pable: Printable)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf = {
+  def apply(pable: Printable)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf =
+    printfWithReset(pable)(sourceInfo, compileOptions)
+
+  private[chisel3] def printfWithReset(
+    pable: Printable
+  )(
+    implicit sourceInfo: SourceInfo,
+    compileOptions:      CompileOptions
+  ): Printf = {
     var printfId: Printf = null
     when(!Module.reset.asBool) {
       printfId = printfWithoutReset(pable)
diff --git a/core/src/main/scala/chisel3/VerificationStatement.scala b/core/src/main/scala/chisel3/VerificationStatement.scala
index 10cece60..a0040d78 100644
--- a/core/src/main/scala/chisel3/VerificationStatement.scala
+++ b/core/src/main/scala/chisel3/VerificationStatement.scala
@@ -48,7 +48,7 @@ object assert extends VerifPrintMacrosDoc {
   )(
     implicit sourceInfo: SourceInfo,
     compileOptions:      CompileOptions
-  ): Assert = macro _applyMacroWithStringMessage
+  ): Assert = macro _applyMacroWithInterpolatorCheck
 
   /** Checks for a condition to be valid in the circuit at all times. If the
     * condition evaluates to false, the circuit simulation stops with an error.
@@ -79,6 +79,32 @@ object assert extends VerifPrintMacrosDoc {
   def apply(cond: Bool)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Assert =
     macro _applyMacroWithNoMessage
 
+  import VerificationStatement._
+
+  /** @group VerifPrintMacros */
+  def _applyMacroWithInterpolatorCheck(
+    c:              blackbox.Context
+  )(cond:           c.Tree,
+    message:        c.Tree,
+    data:           c.Tree*
+  )(sourceInfo:     c.Tree,
+    compileOptions: c.Tree
+  ): c.Tree = {
+    import c.universe._
+    message match {
+      case q"scala.StringContext.apply(..$_).s(..$_)" =>
+        c.warning(
+          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 _ =>
+    }
+    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)"
+  }
+
   /** An elaboration-time assertion. Calls the built-in Scala assert function. */
   def apply(cond: Boolean, message: => String): Unit = Predef.assert(cond, message)
 
@@ -88,8 +114,6 @@ object assert extends VerifPrintMacrosDoc {
   /** Named class for assertions. */
   final class Assert private[chisel3] () extends VerificationStatement
 
-  import VerificationStatement._
-
   /** @group VerifPrintMacros */
   @deprecated(
     "This method has been deprecated in favor of _applyMacroWithStringMessage. Please use the same.",
@@ -215,7 +239,7 @@ object assume extends VerifPrintMacrosDoc {
   )(
     implicit sourceInfo: SourceInfo,
     compileOptions:      CompileOptions
-  ): Assume = macro _applyMacroWithStringMessage
+  ): Assume = macro _applyMacroWithInterpolatorCheck
 
   /** Assumes a condition to be valid in the circuit at all times.
     * Acts like an assertion in simulation and imposes a declarative
@@ -257,6 +281,30 @@ object assume extends VerifPrintMacrosDoc {
   import VerificationStatement._
 
   /** @group VerifPrintMacros */
+  def _applyMacroWithInterpolatorCheck(
+    c:              blackbox.Context
+  )(cond:           c.Tree,
+    message:        c.Tree,
+    data:           c.Tree*
+  )(sourceInfo:     c.Tree,
+    compileOptions: c.Tree
+  ): c.Tree = {
+    import c.universe._
+    message match {
+      case q"scala.StringContext.apply(..$_).s(..$_)" =>
+        c.warning(
+          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 _ =>
+    }
+    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)"
+  }
+
+  /** @group VerifPrintMacros */
   @deprecated(
     "This method has been deprecated in favor of _applyMacroWithStringMessage. Please use the same.",
     "Chisel 3.5"