[Feature #20507] Allow compilation of C extensions with -Wconversion

[flavorjones]

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

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.

This pull request 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.

[shyouhei]

Enclose them using RBIMPL_CAST(). This is a no-op macro in C but saves C++ programs.

[flavorjones]

@nobu @shyouhei Thank you for the review. I've updated the PR to use RBIMPL_CAST.

[casperisfine]

Any way to write a regression test for this? I fear we may re-introduce such warnings without noticing.

[flavorjones]

@casperisfine A regression test would be as easy as running MakeMakefile.append_cflags("-Wconversion") and checking the result in one of the C extensions' extconf files, but I wasn't sure where to stick that or whether it was necessary.

Short term, if there are regressions, I'll see them and then at that point I will surely try to address it.

[flavorjones]

@casperisfine I've added a regression test. Would love feedback.

[byroot]

I've added a regression test. Would love feedback.

I'm really no expert, but the extconf make sense and should prevent regressions from what I can tell. Thank you!

[shyouhei]

LGTM now!