Skip to content
This repository has been archived by the owner on Aug 19, 2024. It is now read-only.

fix: S-interpolator for assert, assume and printf #558

Merged
merged 2 commits into from
Oct 4, 2022

Conversation

SingularityKChen
Copy link
Contributor

To fix the errors in chipsalliance/chisel#2751.

@CLAassistant
Copy link

CLAassistant commented Oct 4, 2022

CLA assistant check
All committers have signed the CLA.

@ekiwi
Copy link
Collaborator

ekiwi commented Oct 4, 2022

Unfortunately this changes the functionality of the code since it will now print out $expected instead of the string representation of expected. @jackkoenig: what is the recommended way to fix this?

Comment on lines +58 to +59
val msg = s"Expected $expected, got %x.\n" // this works around the fact that s".." is forbidden in the assert
assert(reader.io.out === expected.U, msg, reader.io.out)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val msg = s"Expected $expected, got %x.\n" // this works around the fact that s".." is forbidden in the assert
assert(reader.io.out === expected.U, msg, reader.io.out)
assert(reader.io.out === expected.U, cf"Expected ${expected.toString}, got ${reader.io.out}%x.\n")

Copy link
Collaborator

@ekiwi ekiwi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot

@ekiwi ekiwi merged commit 1fb7b18 into ucb-bar:main Oct 4, 2022
@jackkoenig
Copy link
Collaborator

@jackkoenig: what is the recommended way to fix this?

You can either split out the String as you have done, or use cf but explicitly .toString as in the "suggestion" comment I left.

@ekiwi
Copy link
Collaborator

ekiwi commented Oct 4, 2022

You can either split out the String as you have done, or use cf but explicitly .toString as in the "suggestion" comment I left.

I would prefer not to call toString explicitly. What exactly does cf do? Can you link me to the documentation for that (hard to google)

@ekiwi
Copy link
Collaborator

ekiwi commented Oct 4, 2022

I would prefer not to call toString explicitly. What exactly does cf do? Can you link me to the documentation for that (hard to google)

Never mind, I found it: https://www.chisel-lang.org/chisel3/docs/explanations/printing.html
Somehow I thought the Chisel string interpolator was p.

@jackkoenig
Copy link
Collaborator

Until fairly recently, p was the Chisel String interpolator, but in order to add format specifiers, we added cf. Since cf can do anything p can, and it's name makes more sense ("Chisel format string") rather than ("printable"), I figure we should just use cf instead.

@SingularityKChen
Copy link
Contributor Author

SingularityKChen commented Oct 5, 2022

@jackkoenig: what is the recommended way to fix this?

You can either split out the String as you have done, or use cf but explicitly .toString as in the "suggestion" comment I left.

Hi Jack, thanks for your help. But there is one thing that I'm quite confused about. Why do we need to explicitly .toString to the Int type in cf? Dose cf cannot convert Scala types?

In that case, is the following code correct?

val intA: Int = 10
val uIntB: UInt = intA.U
print(cf"intA=${intA.toString} and uIntB=$uIntB")
printf(cf"" + s"intA=$intA and " + cf"uIntB=$uIntB")  // the first cf is used to avoid compile error

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants