New subject: [ruby-core:118008] [Ruby master Misc#20507] Allow C extensions to be compiled cleanly with the `-Wsign-conversion` warning option

24 May 2024

      Issue #20507 has been reported by mdalessio (Mike Dalessio).

----------------------------------------
Misc #20507: Allow C extensions to be compiled cleanly with the `-Wsign-conversion` warning option
https://bugs.ruby-lang.org/issues/20507

* Author: mdalessio (Mike Dalessio)
* Status: Open
----------------------------------------
As a maintainer of several C extensions, I like to compile my code with some of the common [Warning Options](https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html) to help maintain code quality and safety.

One of the options that is currently very difficult to use is `-Wsign-conversion`, because some of the Ruby public header files contain code that will generate warnings. So many warnings are printed when compiling against Ruby's header files that it's hard to see any warnings from my code.

As an example, compiling just one file in Nokogiri with this flag enabled will print:

```
.../include/ruby-3.4.0+0/ruby/internal/special_consts.h: In function ‘RB_TEST’:
.../include/ruby-3.4.0+0/ruby/internal/special_consts.h:159:16: warning: unsigned conversion from ‘int’ to ‘VALUE’ {aka ‘long unsigned int’} changes value from ‘-5’ to ‘18446744073709551611’ [-Wsign-conversion]
  159 |     return obj & ~RUBY_Qnil;
      |                ^
.../include/ruby-3.4.0+0/ruby/internal/special_consts.h: In function ‘RB_NIL_OR_UNDEF_P’:
.../include/ruby-3.4.0+0/ruby/internal/special_consts.h:229:24: warning: unsigned conversion from ‘int’ to ‘VALUE’ {aka ‘const long unsigned int’} changes value from ‘-33’ to ‘18446744073709551583’ [-Wsign-conversion]
  229 |     const VALUE mask = ~(RUBY_Qundef ^ RUBY_Qnil);
      |                        ^
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h: In function ‘RB_INT2FIX’:
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h:117:29: warning: conversion to ‘long unsigned int’ from ‘long int’ may change the sign of the result [-Wsign-conversion]
  117 |     const unsigned long j = i;
      |                             ^
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h:119:29: warning: conversion to ‘long int’ from ‘long unsigned int’ may change the sign of the result [-Wsign-conversion]
  119 |     const long          l = k;
      |                             ^
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h:121:29: warning: conversion to ‘VALUE’ {aka ‘const long unsigned int’} from ‘long int’ may change the sign of the result [-Wsign-conversion]
  121 |     const VALUE         n = m;
      |                             ^
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h: In function ‘rbimpl_fix2long_by_idiv’:
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h:169:28: warning: conversion to ‘long int’ from ‘VALUE’ {aka ‘long unsigned int’} may change the sign of the result [-Wsign-conversion]
  169 |     const SIGNED_VALUE y = x - RUBY_FIXNUM_FLAG;
      |                            ^
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h: In function ‘rbimpl_fix2long_by_shift’:
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h:196:28: warning: conversion to ‘long int’ from ‘VALUE’ {aka ‘long unsigned int’} may change the sign of the result [-Wsign-conversion]
  196 |     const SIGNED_VALUE y = x;
      |                            ^
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h: In function ‘rb_fix2ulong’:
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h:255:12: warning: conversion to ‘long unsigned int’ from ‘long int’ may change the sign of the result [-Wsign-conversion]
  255 |     return rb_fix2long(x);
      |            ^~~~~~~~~~~~~~
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h: In function ‘rb_ulong2num_inline’:
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h:326:28: warning: conversion to ‘long int’ from ‘long unsigned int’ may change the sign of the result [-Wsign-conversion]
  326 |         return RB_LONG2FIX(v);
      |                            ^
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long_long.h: In function ‘rb_num2ull_inline’:
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long.h:53:22: warning: conversion to ‘long long unsigned int’ from ‘long int’ may change the sign of the result [-Wsign-conversion]
   53 | #define RB_FIX2LONG  rb_fix2long          /**< @alias{rb_fix2long} */
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/long_long.h:130:16: note: in expansion of macro ‘RB_FIX2LONG’
  130 |         return RB_FIX2LONG(x);
      |                ^~~~~~~~~~~
In file included from .../include/ruby-3.4.0+0/ruby/internal/arithmetic.h:37:
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/st_data_t.h: In function ‘RB_ST2FIX’:
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/st_data_t.h:61:22: warning: conversion to ‘long int’ from ‘st_data_t’ {aka ‘long unsigned int’} may change the sign of the result [-Wsign-conversion]
   61 |     SIGNED_VALUE x = i;
      |                      ^
.../include/ruby-3.4.0+0/ruby/internal/arithmetic/st_data_t.h:72:24: warning: conversion to ‘long int’ from ‘long unsigned int’ may change the sign of the result [-Wsign-conversion]
   72 |     return RB_LONG2FIX(y);
      |                        ^
In file included from .../include/ruby-3.4.0+0/ruby/ruby.h:42:
.../include/ruby-3.4.0+0/ruby/internal/memory.h: In function ‘rb_alloc_tmp_buffer2’:
.../include/ruby-3.4.0+0/ruby/internal/memory.h:646:56: warning: conversion to ‘size_t’ {aka ‘long unsigned int’} from ‘long int’ may change the sign of the result [-Wsign-conversion]
  646 |     const size_t total_size = rbimpl_size_mul_or_raise(count, elsize);
      |                                                        ^~~~~
In file included from .../include/ruby-3.4.0+0/ruby/internal/encoding/ctype.h:27,
                 from .../include/ruby-3.4.0+0/ruby/encoding.h:22,
                 from ../../../../ext/nokogiri/nokogiri.h:79:
.../include/ruby-3.4.0+0/ruby/internal/encoding/encoding.h: In function ‘RB_ENCODING_SET_INLINED’:
.../include/ruby-3.4.0+0/ruby/internal/encoding/encoding.h:83:28: warning: conversion to ‘VALUE’ {aka ‘long unsigned int’} from ‘int’ may change the sign of the result [-Wsign-conversion]
   83 |     VALUE f = /* upcast */ encindex;
      |                            ^~~~~~~~
```

Compiling all 35 files in Nokogiri's C extension means these warnings are repeated another 34 times in a full build. This makes it difficult to see any conversion errors in my own code.

I'm about to open a pull request which adds casts where there are intentional, known-safe integer conversions occurring that are triggering these warnings.

If merged, then C extension owners can add this to their `extconf.rb`:

```
append_cflags("-Wconversion")
```

and then any warnings generated are from the C extension code, and not from Ruby's header files.

-- 
https://bugs.ruby-lang.org/

[ruby-core:118007] [Ruby master Misc#20507] Allow C extensions to be compiled cleanly with the `-Wsign-conversion` warning option

mdalessio (Mike Dalessio)

mdalessio (Mike Dalessio)

shyouhei (Shyouhei Urabe)

nobu (Nobuyoshi Nakada)

shyouhei (Shyouhei Urabe)

tags

participants (3)