Misc #20507: Allow C extensions to be compiled cleanly with the `-Wsign-conversion` warning option

• Status: Closed
• Assignee: -

[mdalessio (Mike Dalessio)]

As a maintainer of several C extensions, I like to compile my code with some of the common Warning Options 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.

[mdalessio (Mike Dalessio)]

Pull request is https://github.com/ruby/ruby/pull/10843

[shyouhei (Shyouhei Urabe)]

These type conversions are intentional (yes, we are changing the sign of the results). GCC tends to be more meddlesome than useful. Also note that -Wsign-conversion is not included in any of -Wall, -Wextra, or -Wpedantic.

That said yes, I agree that any warnings from something out of your control is just annoying. It would be nice if our public headers are friendly to anyone. +1 to suppress them.

[nobu (Nobuyoshi Nakada)]

RBIMPL_CAST not needed?

[shyouhei (Shyouhei Urabe)]

Needed. The pull request needs an approval before merged.

[mdalessio (Mike Dalessio)]

• Status changed from Open to Closed

Applied in changeset git|1b8ba1551b26fac906998e34fd2af3f82b433469.

---

Allow compilation of C extensions with -Wconversion

C extension maintainers can now compile with this warning option and
the Ruby header files will generate no warnings.

[Feature #20507]