-
Notifications
You must be signed in to change notification settings - Fork 615
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
Add BigInt / Int to Bool conversion (0.B, 1.B) #913
Conversation
Doesn't this mean |
Yeah. It would be nice if there was a way to make that type-safe, but there's a trade-off between syntax and error type in this implementation. We can discuss at the meeting today. Macros might be able to detect something like |
No objections here. I don’t think it’s especially problematic that 2.B is a runtime error. |
retest this please |
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.
The implementation looks good to me.
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.
Looks good except I think it could use a better error message
/** Int to Bool conversion, allowing compact syntax like 1.B and 0.B | ||
*/ | ||
def B: Bool = { | ||
require(bigint == 0 || bigint == 1) |
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.
I would provide an explicit message so that people who try 3.B
get something more than requirement failed
415e89a
to
b827f77
Compare
I think there are cases where it's cleaner to use
0.B
and1.B
instead offalse.B
andtrue.B
, especially as testers is going to use chisel literals.(the use case is in testing interface code, where signal levels are usually specified as 0 and 1 instead of false and true)
Type of change: other enhancement
Impact: API addition (no impact on existing code)
Development Phase: proposal, implementation
Release Notes