-
Notifications
You must be signed in to change notification settings - Fork 603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Error on S-interpolator usage for assert, assume and printf #2751
Error on S-interpolator usage for assert, assume and printf #2751
Conversation
I think this requires some fixes to the printable docs? Or can we say something there to show people the warning/error they will get if they try to printf(s"...")? |
649345d
to
00cce9e
Compare
Co-authored-by: Megan Wachs <megan@sifive.com>
Update a callsite. Rename function in Printf.scala to match the naming convention in the file. Undo call by name change to preserve binary compatibility.
00cce9e
to
b7fdc43
Compare
Better docs for the error Update error message to not mention the p-interpolator
b7fdc43
to
7a11bd4
Compare
This is not marked with Milestone |
} | ||
``` | ||
|
||
Instead, use Chisel's `cf` interpolator as in the following examples: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice :)
This is to be backported to 3.5.x as a warning instead of an error
Contributor Checklist
docs/src
?Type of Improvement
API Impact
Current
assert
,assume
andprintf
APIs allow the Scala s-interpolator for formatting messages. This often leads to user errors where the Scala object is printed instead of the Chisel object, which leads to the messages being generally unhelpful.After this update, using s-interpolators will throw an error about deprecation.
Backend Code Generation Impact
N/A
Desired Merge Strategy
Release Notes
Using Scala s-interpolators now throws an error recommending a shift to the cf interpolator.
Reviewer Checklist (only modified by reviewer)
3.4.x
, [small] API extension:3.5.x
, API modification or big change:3.6.0
)?Enable auto-merge (squash)
, clean up the commit message, and label withPlease Merge
.Create a merge commit
.