Skip to content

Commit 39327df

Browse files
Fix Printf macro to catch s-interpolator usages in Scala 2.13 (#3143) (#3157)
Co-authored-by: Jack Koenig <koenig@sifive.com> (cherry picked from commit f3d0f22) Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent dc54a98 commit 39327df

File tree

3 files changed

+37
-41
lines changed

3 files changed

+37
-41
lines changed

core/src/main/scala/chisel3/Printf.scala

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,38 @@ object printf {
3636
formatIn.map(escaped).mkString("")
3737
}
3838

39+
private[chisel3] def _checkFormatString(c: blackbox.Context)(fmt: c.Tree): Unit = {
40+
import c.universe._
41+
42+
val errorString = "The s-interpolator prints the Scala .toString of Data objects rather than the value " +
43+
"of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
44+
"an elaboration time print, use println."
45+
46+
// Error on Data in the AST by matching on the Scala 2.13 string
47+
// interpolation lowering to concatenation
48+
def throwOnChiselData(x: c.Tree): Unit = x match {
49+
case q"$x+$y" => {
50+
if (x.tpe <:< typeOf[chisel3.Data] || y.tpe <:< typeOf[chisel3.Data]) {
51+
c.error(c.enclosingPosition, errorString)
52+
} else {
53+
throwOnChiselData(x)
54+
throwOnChiselData(y)
55+
}
56+
}
57+
case _ =>
58+
}
59+
throwOnChiselData(fmt)
60+
61+
fmt match {
62+
case q"scala.StringContext.apply(..$_).s(..$_)" =>
63+
c.error(
64+
c.enclosingPosition,
65+
errorString
66+
)
67+
case _ =>
68+
}
69+
}
70+
3971
/** Named class for [[printf]]s. */
4072
final class Printf private[chisel3] (val pable: Printable) extends VerificationStatement
4173

@@ -87,16 +119,7 @@ object printf {
87119
compileOptions: c.Tree
88120
): c.Tree = {
89121
import c.universe._
90-
fmt match {
91-
case q"scala.StringContext.apply(..$_).s(..$_)" =>
92-
c.error(
93-
c.enclosingPosition,
94-
"The s-interpolator prints the Scala .toString of Data objects rather than the value " +
95-
"of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
96-
"an elaboration time print, use println."
97-
)
98-
case _ =>
99-
}
122+
_checkFormatString(c)(fmt)
100123
val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("printfWithReset"))
101124
q"$apply_impl_do(_root_.chisel3.Printable.pack($fmt, ..$data))($sourceInfo, $compileOptions)"
102125
}

core/src/main/scala/chisel3/VerificationStatement.scala

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -92,16 +92,7 @@ object assert extends VerifPrintMacrosDoc {
9292
compileOptions: c.Tree
9393
): c.Tree = {
9494
import c.universe._
95-
message match {
96-
case q"scala.StringContext.apply(..$_).s(..$_)" =>
97-
c.error(
98-
c.enclosingPosition,
99-
"The s-interpolator prints the Scala .toString of Data objects rather than the value " +
100-
"of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
101-
"an elaboration time check, call assert with a Boolean condition."
102-
)
103-
case _ =>
104-
}
95+
printf._checkFormatString(c)(message)
10596
val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable"))
10697
q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some(_root_.chisel3.Printable.pack($message, ..$data)))($sourceInfo, $compileOptions)"
10798
}
@@ -273,16 +264,7 @@ object assume extends VerifPrintMacrosDoc {
273264
compileOptions: c.Tree
274265
): c.Tree = {
275266
import c.universe._
276-
message match {
277-
case q"scala.StringContext.apply(..$_).s(..$_)" =>
278-
c.error(
279-
c.enclosingPosition,
280-
"The s-interpolator prints the Scala .toString of Data objects rather than the value " +
281-
"of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
282-
"an elaboration time check, call assert with a Boolean condition."
283-
)
284-
case _ =>
285-
}
267+
printf._checkFormatString(c)(message)
286268
val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable"))
287269
q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some(_root_.chisel3.Printable.pack($message, ..$data)))($sourceInfo, $compileOptions)"
288270
}

docs/src/explanations/printing.md

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,25 +15,16 @@ Chisel provides the `printf` function for debugging purposes. It comes in two fl
1515

1616
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).
1717

18-
Note that the Scala s-interpolator is not supported in Chisel.
19-
Future versions of Chisel will throw an error when using the s-interpolator
20-
within a printf.
18+
Note that the Scala s-interpolator is not supported in Chisel constructs and will throw an error:
2119

2220
```scala mdoc:invisible
2321
import chisel3._
24-
25-
//TODO: Make the following Module fail when using s-interpolator in a printf
26-
// This used to fail under 2.12 but the macro that implemented this does
27-
// not work properly in Scala 2.13
28-
// When fixed add mdoc:fail to following ```scala block and clean up
2922
```
3023

31-
```scala
24+
```scala mdoc:fail
3225
class MyModule extends Module {
3326
val in = IO(Input(UInt(8.W)))
3427
printf(s"in = $in\n")
35-
^
36-
+----- Do NOT use the s-interpolator in a printf
3728
}
3829
```
3930

0 commit comments

Comments
 (0)