Fix -Wsign-compare gcc warning #833
Conversation
gcc warns about this code:
`warning: comparison of integer expressions of different signedness: 'size_t' {aka 'long unsigned int'} and 'int' [-Wsign-compare]`
From what I can tell, this is a difference between clang and gcc, as
explored [on godbolt](https://godbolt.org/z/T3TKsdvTr)
|
I know that the purification ritual comment mentions Please also let me know if you have a better way you'd like this formatted. There are not a lot (any?) examples of multi-line for loops that I could find. |
| for (size_t i = 1; | ||
| i < fc->nargs - static_cast<size_t>(has_var_kwargs); ++i) { |
There was a problem hiding this comment.
Since int can represent the entire range of values of the original types on the right, integer promotion converts each value to int. Then subtraction results in an int. So, may I suggest:
for (int i = 1; i < fc->nargs - has_var_kwargs; ++i) {
I feel this is more natural/readable (and it fits on one line). As a bonus, the generated code might be better. On Compiler Explorer, I think it's nicer to uncheck the "Intel asm syntax" box. Note that the int code is smaller using 32-bit integers than for 64-bit size_t, because addq takes one more byte than addl, etc.
64bit: 48 83 c3 01 add $0x1,%rbx ==> 32bit: 83 c3 01 add $0x1,%ebx
64bit: 48 39 eb cmp %rbp,%rbx ==> 32bit: 39 eb cmp %ebp,%ebx
|
I committed a fix directly. I am not a fan of |
|
Sounds good. Thanks for the fix! |
gcc warns about this code:
warning: comparison of integer expressions of different signedness: 'size_t' {aka 'long unsigned int'} and 'int' [-Wsign-compare]From what I can tell, this is a difference between clang and gcc, as explored on godbolt