Silence signedness change through implicit conversion error #880
Conversation
src/event/event_epoll.c
Outdated
| @@ -89,7 +89,7 @@ DISPATCH_ALWAYS_INLINE | |||
| static inline uint32_t | |||
| _dispatch_muxnote_armed_events(dispatch_muxnote_t dmn) | |||
| { | |||
| return dmn->dmn_events & ~dmn->dmn_disarmed_events; | |||
| return dmn->dmn_events & ~(uint32_t)(dmn->dmn_disarmed_events); | |||
There was a problem hiding this comment.
I think that casting to uint16_t is better as that is the declaration. That should avoid any unintentional zexting.
There was a problem hiding this comment.
You mean s/uint32_t/uint16_t/? Or (uint16_t)~dmn->dmn_disarmed_events? The former doesn’t help. I don’t really get which part is causing the conversion. Since the latter works, my guess is it started picking a signed negation overload.
There was a problem hiding this comment.
Why is it thinking that the member is 32 bit? It’s trying to highlight the implicit integer promotion. We just want to ensure the zext instead of a sext.
There was a problem hiding this comment.
The error says 'int' to 'uint32_t'. I’m curious where the int comes from. If int is what the selected negation returns, then (uint16_t)~dmn->dmn_disarmed_events seems like it would cause a double conversion, whereas the current change appears to make it select a negation that returns an uint32_t.
There was a problem hiding this comment.
It really is the ~:
uint16_t x = dmn->dmn_disarmed_events;
uint16_t nx = ~x;
return dmn->dmn_events & nx;error: implicit conversion loses integer precision: 'int' to 'uint16_t' (aka 'unsigned short') [-Werror,-Wimplicit-int-conversion]
93 | uint16_t nx = ~x;
| ~~ ^~
~(uint16_t)x produces int, and ~(uint32_t)x produces uint32_t 🤷🏼♂️
Anyway, if you stand by your original suggestion, I would appreciate some clarity in regards to the subexpression to be casted.
I think that casting to
uint16_tis better as that is the declaration.
There was a problem hiding this comment.
Ah right ... the result of ~ would be int because it will widen to the 32-bit int. I'd say we can spell this as: ~(uint32_t)(uint16_t)(dmn->dmn_events) then. The first cast is meaningless but is meant to ensure that we are convert an unsigned to unsigned value to ensure the zext over the sext.
There was a problem hiding this comment.
Switched to decltype(dmn->dmn_events) after discussing offline with Ben.
AnthonyLatsis
force-pushed
the
linux-20250402
branch
from
b7634b7 to
611ed9a
Compare
|
@swift-ci please test |
|
@swift-ci please test Linux platform |
|
Oh, hadn't updated yet so this is just going to fail again 😅 |
The error pops up when using the rebranch Clang (stable/20250402): ``` /home/build-user/swift-corelibs-libdispatch/src/event/event_epoll.c:92:27: error: implicit conversion changes signedness: 'int' to 'uint32_t' (aka 'unsigned int') [-Werror,-Wsign-conversion] 92 | return dmn->dmn_events & ~dmn->dmn_disarmed_events; | ~ ^~~~~~~~~~~~~~~~~~~~~~~~~ ```
AnthonyLatsis
force-pushed
the
linux-20250402
branch
from
611ed9a to
38872e2
Compare
|
@swift-ci please test |
|
@swift-ci please test |
The error pops up when using the rebranch Clang (stable/20250402):