-
Notifications
You must be signed in to change notification settings - Fork 1k
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 static assertion that uint32_t is unsigned int or wider #818
Add static assertion that uint32_t is unsigned int or wider #818
Conversation
|
||
/* No integer promotion for uint32_t. This ensures that we can multiply uintXX_t values where XX >= 32 | ||
without signed overflow, which would be undefined behaviour. */ | ||
(UINT_MAX <= UINT32_MAX) && |
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 think INT_MAX < UINT32_MAX
would more directly capture your intention here; however given the other rules of C, I do admit that what you have written ends up being equivalent.
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.
Yeah, I think our intuition is just different. My thought was that if uint32_t
is at least as large as unsigned int
, then no promotion can happen at all, neither to signed int
nor to unsigned int
.
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 think of the rule as if every value of a type can fit within a signed int
, then it is subject to integer promotion.
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.
Both seem reasonable and harmless 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.
Wanna ACK then?
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.
ACK c0041b5
utACK c0041b5 |
Summary: This is a backport of secp256k1 [[bitcoin-core/secp256k1#818 | PR818]] Depends on D7632 Test Plan: ninja check-secp256k1 Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D7635
Summary: This is a backport of secp256k1 [[bitcoin-core/secp256k1#818 | PR818]] Depends on D7632 Test Plan: ninja check-secp256k1 Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D7635
Solves one item in #792 .