Hello,
Since Wconversion got enabled on Genode master branch I'm wondering if there are any plans WRT Wsign-conversion warnings? I presonally find it a bit strange GCC Wconversion does not also enable Wsign-conversion. IMO implicit conversions between signed and unsigned types are as often as bad as implicit conversion between types of different size.
/ptw
Hi Piotr,
On 18.12.21 22:32, Piotr Tworek wrote:
Since Wconversion got enabled on Genode master branch I'm wondering if there are any plans WRT Wsign-conversion warnings? I presonally find it a bit strange GCC Wconversion does not also enable Wsign-conversion.
this surprises me as well. I have silently assumed that the latter is implied. Thank you for pointing that out.
IMO implicit conversions between signed and unsigned types are as often as bad as implicit conversion between types of different size.
I wholeheartedly agree.
I'm all for enabling -Wsign-conversion in addition. However, I'm afraid that I personally need a little break from the topic. I have seen your latest pull request [1]. Such improvements - especially with the rationale so well described as in your commits - are always very much appreciated.
[1] https://github.com/genodelabs/genode/pull/4354
While working on the -Wconversion topic, I actually wondered about the use cases of signed integers throughout Genode's base framework and grep'ed for them out of curiosity. A few niche uses (like the balancing of AVL trees) notwithstanding, I found that signed integers are almost never deliberately used. It is tempting to relieve us from the uncertainties related to signed integers by removing the remaining instances from the base framework.
Cheers Norman
On Tue, 2021-12-21 at 12:03 +0100, Norman Feske wrote:
Hi Piotr,
On 18.12.21 22:32, Piotr Tworek wrote:
Since Wconversion got enabled on Genode master branch I'm wondering if there are any plans WRT Wsign-conversion warnings? I presonally find it a bit strange GCC Wconversion does not also enable Wsign- conversion.
this surprises me as well. I have silently assumed that the latter is implied. Thank you for pointing that out.
No problem. Some time ago I also thought Wconversion implies all conversion like warnings. Unfortunately its not the case in GCC.
IMO implicit conversions between signed and unsigned types are as often as bad as implicit conversion between types of different size.
I wholeheartedly agree.
I'm all for enabling -Wsign-conversion in addition. However, I'm afraid that I personally need a little break from the topic. I have seen your latest pull request [1]. Such improvements - especially with the rationale so well described as in your commits - are always very much appreciated.
Yep, the PR is enough to compile current master with clang 13 and -Wno- sign-conversion. One of the fixes also touches some trivial sign- conversion warnings that did not require too much time to investigate.
While working on the -Wconversion topic, I actually wondered about the use cases of signed integers throughout Genode's base framework and grep'ed for them out of curiosity. A few niche uses (like the balancing of AVL trees) notwithstanding, I found that signed integers are almost never deliberately used. It is tempting to relieve us from the uncertainties related to signed integers by removing the remaining instances from the base framework.
I don't know the framework as well as You do, but I did notice a few places where signed integers seem like a bit odd choice. In all such cases clang complains about such signed values being implcitly converted to unsigned type at some point. My assumption is GCC will issue the same warnings when Wsign-conversion is explcitly enabled. I have some changes in my git stash that adress some of those warnings. I'll see if I can find the time to make a few proper commits out of those. I probably won't have the time to fix all Wsign-conversion problems, but I might at least reduce their number a bit.
/ptw
-
Norman Feske -
Piotr Tworek