Add support for Ruby 2.7 by treitmayr · Pull Request #1692 · swig/swig

treitmayr · 2019-12-29T20:51:30Z

This commit fixes the signatures of various callback methods
based on https://github.com/ruby/ruby/pull/2404/files?file-filters%5B%5D=.h
and cleans up the macro definitions used for casting callbacks.

Note that the transparent version of the macro RUBY_METHOD_FUNC
is currently masked behind RUBY_DEVEL, see commit
ruby/ruby@1d91fea
In order to still support strict signature checking and prevent
nasty deprecation warnings, the use of RUBY_METHOD_FUNC had to
be replaced with VALUEFUNC.

This commit also fixes issue #1689.

anatol · 2019-12-30T16:55:54Z

Thank you very much for the patch. I backported it to SWIG3 (used by libsigrok due to https://sigrok.org/bugzilla/show_bug.cgi?id=1468) and was able to compile libsigrok. There are few warnings that you might be interested in

bindings/ruby/classes_wrap.cpp: In function ‘std::map<std::__cxx11::basic_string<char>, Glib::VariantBase> hash_to_map_options(VALUE, std::map<std::__cxx11::basic_string<char>, std::shared_ptr<sigrok::Option> >)’:
bindings/ruby/classes_wrap.cpp:10582:75: warning: ‘void ruby::backward::cxxanyargs::rb_hash_foreach(VALUE, int (*)(...), VALUE)’ is deprecated: Use of ANYARGS in this function is deprecated [-Wdeprecated-declarations]
10582 |     rb_hash_foreach(hash, (int (*)(ANYARGS))convert_option, (VALUE)&params);
      |                                                                           ^
In file included from /usr/include/ruby-2.7.0/ruby/ruby.h:2863,
                 from /usr/include/ruby-2.7.0/ruby.h:33,
                 from bindings/ruby/classes_wrap.cpp:879:
/usr/include/ruby-2.7.0/ruby/backward/cxxanyargs.hpp:412:1: note: declared here
  412 | rb_hash_foreach(VALUE q, int_type *w, VALUE e)
      | ^~~~~~~~~~~~~~~
bindings/ruby/classes_wrap.cpp:10582:75: warning: ‘void ruby::backward::cxxanyargs::rb_hash_foreach(VALUE, int (*)(...), VALUE)’ is deprecated: Use of ANYARGS in this function is deprecated [-Wdeprecated-declarations]
10582 |     rb_hash_foreach(hash, (int (*)(ANYARGS))convert_option, (VALUE)&params);
      |                                                                           ^
In file included from /usr/include/ruby-2.7.0/ruby/ruby.h:2863,
                 from /usr/include/ruby-2.7.0/ruby.h:33,
                 from bindings/ruby/classes_wrap.cpp:879:
/usr/include/ruby-2.7.0/ruby/backward/cxxanyargs.hpp:412:1: note: declared here
  412 | rb_hash_foreach(VALUE q, int_type *w, VALUE e)
      | ^~~~~~~~~~~~~~~
bindings/ruby/classes_wrap.cpp: In function ‘std::vector<std::shared_ptr<sigrok::HardwareDevice> > sigrok_Driver__scan__SWIG_0(sigrok::Driver*, VALUE)’:
bindings/ruby/classes_wrap.cpp:10854:89: warning: ‘void ruby::backward::cxxanyargs::rb_hash_foreach(VALUE, int (*)(...), VALUE)’ is deprecated: Use of ANYARGS in this function is deprecated [-Wdeprecated-declarations]
10854 |         rb_hash_foreach(kwargs, (int (*)(ANYARGS))convert_option_by_key, (VALUE)&options);
      |                                                                                         ^
In file included from /usr/include/ruby-2.7.0/ruby/ruby.h:2863,
                 from /usr/include/ruby-2.7.0/ruby.h:33,
                 from bindings/ruby/classes_wrap.cpp:879:
/usr/include/ruby-2.7.0/ruby/backward/cxxanyargs.hpp:412:1: note: declared here
  412 | rb_hash_foreach(VALUE q, int_type *w, VALUE e)
      | ^~~~~~~~~~~~~~~
bindings/ruby/classes_wrap.cpp:10854:89: warning: ‘void ruby::backward::cxxanyargs::rb_hash_foreach(VALUE, int (*)(...), VALUE)’ is deprecated: Use of ANYARGS in this function is deprecated [-Wdeprecated-declarations]
10854 |         rb_hash_foreach(kwargs, (int (*)(ANYARGS))convert_option_by_key, (VALUE)&options);
      |                                                                                         ^
In file included from /usr/include/ruby-2.7.0/ruby/ruby.h:2863,
                 from /usr/include/ruby-2.7.0/ruby.h:33,
                 from bindings/ruby/classes_wrap.cpp:879:
/usr/include/ruby-2.7.0/ruby/backward/cxxanyargs.hpp:412:1: note: declared here
  412 | rb_hash_foreach(VALUE q, int_type *w, VALUE e)
      | ^~~~~~~~~~~~~~~
  CXXLD    bindings/cxx/libsigrokcxx.la
ar: `u' modifier ignored since `D' is the default (see `U')
  GEN      bindings/java/libsigrok_java_core_classes.so
  GEN      bindings/python/timestamp
sigrok/core/classes_wrap.cpp:1844:36: warning: cast between incompatible function types from ‘PyObject* (*)(PyObject*)’ {aka ‘_object* (*)(_object*)’} to ‘PyCFunction’ {aka ‘_object* (*)(_object*, _object*)’} [-Wcast-function-type]
 1844 |   {(char *)"disown",  (PyCFunction)SwigPyObject_disown,  METH_NOARGS,  (char *)"releases ownership of the pointer"},
      |                                    ^~~~~~~~~~~~~~~~~~~
sigrok/core/classes_wrap.cpp:1845:36: warning: cast between incompatible function types from ‘PyObject* (*)(PyObject*)’ {aka ‘_object* (*)(_object*)’} to ‘PyCFunction’ {aka ‘_object* (*)(_object*, _object*)’} [-Wcast-function-type]
 1845 |   {(char *)"acquire", (PyCFunction)SwigPyObject_acquire, METH_NOARGS,  (char *)"acquires ownership of the pointer"},
      |                                    ^~~~~~~~~~~~~~~~~~~~
sigrok/core/classes_wrap.cpp:1848:36: warning: cast between incompatible function types from ‘PyObject* (*)(PyObject*)’ {aka ‘_object* (*)(_object*)’} to ‘PyCFunction’ {aka ‘_object* (*)(_object*, _object*)’} [-Wcast-function-type]
 1848 |   {(char *)"next",    (PyCFunction)SwigPyObject_next,    METH_NOARGS,  (char *)"returns the next 'this' object"},
      |                                    ^~~~~~~~~~~~~~~~~
sigrok/core/classes_wrap.cpp:1849:36: warning: cast between incompatible function types from ‘PyObject* (*)(SwigPyObject*)’ {aka ‘_object* (*)(SwigPyObject*)’} to ‘PyCFunction’ {aka ‘_object* (*)(_object*, _object*)’} [-Wcast-function-type]
 1849 |   {(char *)"__repr__",(PyCFunction)SwigPyObject_repr,    METH_NOARGS,  (char *)"returns object representation"},
      |                                    ^~~~~~~~~~~~~~~~~
sigrok/core/classes_wrap.cpp: In function ‘PyTypeObject* SwigPyObject_TypeOnce()’:
sigrok/core/classes_wrap.cpp:2007:5: warning: missing initializer for member ‘_typeobject::tp_vectorcall’ [-Wmissing-field-initializers]
 2007 |     };
      |     ^
sigrok/core/classes_wrap.cpp:2007:5: warning: missing initializer for member ‘_typeobject::tp_print’ [-Wmissing-field-initializers]
sigrok/core/classes_wrap.cpp: In function ‘PyTypeObject* SwigPyPacked_TypeOnce()’:
sigrok/core/classes_wrap.cpp:2194:5: warning: missing initializer for member ‘_typeobject::tp_vectorcall’ [-Wmissing-field-initializers]
 2194 |     };
      |     ^
sigrok/core/classes_wrap.cpp:2194:5: warning: missing initializer for member ‘_typeobject::tp_print’ [-Wmissing-field-initializers]
  CXXLD    bindings/ruby/sigrok.so
sigrok/core/classes_wrap.cpp: In function ‘PyTypeObject* swig_varlink_type()’:
sigrok/core/classes_wrap.cpp:61564:7: warning: missing initializer for member ‘_typeobject::tp_vectorcall’ [-Wmissing-field-initializers]
61564 |       };
      |       ^

treitmayr · 2019-12-30T18:00:18Z

The deprecation warnings regarding rb_hash_foreach result from code in your classes.i file. There you need to specify the second argument as INT_ANYARGS_FUNC(convert_option) instead of (int (*)(ANYARGS))convert_option and the warning should be fixed. The same applies to convert_option_by_key.
The remaining warnings have nothing to do with the Ruby version, right?

treitmayr · 2019-12-30T18:04:04Z

Is there a way to find out when a certain Ruby version is supported by rvm in the Travis CI? Or shall I remove the new entry in .travis.yml for now?

wsfulton · 2019-12-30T23:25:11Z

.travis.yml

+    - compiler: gcc
+      os: linux
+      env: SWIGLANG=ruby VER=2.7.0
+      sudo: required
+      dist: xenial


I've added in ruby-2.7 testing on Travis to master in 5259082. Can you please remove the changes to .travis.yml and push an updated patch? Then we can then see how your patch performs using 2.7.0p0 on Travis. Thanks.

Actually Travis ran its tests just fine 3 hours ago with my newly committed change (where "2.7.0" instead of "2.7" is requested), see https://travis-ci.org/swig/swig/jobs/631045861?utm_medium=notification&utm_source=github_status .

Sorry for not stating this good news already. How shall we proceed now?

Nice one, I don't know how you found that was supported as rvm list known doesn't include 2.7.0p0, just 2.7[.0-preview1]. Anyway, either approach tests what we want which is good.

Just a gut feeling that there might be some fallback for fully specified version strings. However the final patch does not include that one in favor of your more elaborate solution and to resolve the conflicts.

This commit fixes the signatures of various callback methods and cleans up the macro definitions used for casting callbacks. Note that the transparent version of the macro RUBY_METHOD_FUNC is currently masked behind RUBY_DEVEL, see commit ruby/ruby@1d91fea In order to still support strict signature checking and prevent nasty deprecation warnings, the use of RUBY_METHOD_FUNC had to be replaced with VALUEFUNC.

treitmayr · 2019-12-31T17:08:25Z

Now I also adapted the code for Ruby global variables for Ruby 2.7 (and it actually became shorter and hopefully easier to read), and fixed the excessive white spaces in the macro definitions (therefore the second push).
If the CI build has passed (some time next year) then this will be my new proposal.

wsfulton · 2020-01-03T20:22:52Z

I know the existing macros PROTECTFUNC VALUEFUNC VOIDFUNC are polluting the global namespace, but the new ones VOID_ANYARGS_FUNC and INT_ANYARGS_FUNC ought to be prefixed with SWIG_RUBY_ so as to not pollute the global namespace. I don't know the history of the existing macro names, but we can't change them now as this will go into a patch release of SWIG, but we could rename them at a later date.

treitmayr · 2020-01-03T20:27:30Z

Ok, makes sense, I will change it.
(BTW, I guess in order to have a clean history I will rewrite the original commit instead of adding one!? Just to make sure I comply to this project's rules...)

wsfulton · 2020-01-03T22:07:28Z

It really doesn't matter in this case.

wsfulton · 2020-01-03T22:59:58Z

Lib/ruby/rubyhead.swg

@@ -110,26 +110,18 @@
 * can be passed as an argument to API functions like Data_Wrap_Struct()
 * and Data_Make_Struct().
 */


The comment above needs updating. I think the 1st two lines should be replaced with something like:

The following macros are for backwards compatibility and using strictly correct function signatures in C++
when Ruby 2.7 removed ANYARGS and replaced callback function signatures with explicit types.

Is this your understanding as well? Feel free to reword.

True, I missed that, but I will have to postpone the change until tomorrow...

Fixed it, and learned something about "function declarators with empty parentheses" (https://stackoverflow.com/a/20835620) used in the Ruby C API for C (not C++) extensively before version 2.7. That's why there is this additional condition regarding C++ in the macro definition block.

The macros for casting function pointers are now fully described and also clarify why the macros act transparently for C even before Ruby 2.7. In addition, an "if (CPlusPlus)" was removed in the code generator for global variables in order to keep the distinction between C and C++ in one place, which is at the definition of said macros.

Because swig 4.0.1 doesn't support Ruby 2.7 yet, stop testing Ruby 2.7. See also: swig/swig#1692

treitmayr mentioned this pull request Dec 30, 2019

Ruby 2.7 modified 'rb_define_virtual_variable' signature #1689

Closed

treitmayr force-pushed the ruby2.7-support branch from 4fc9ced to 4105416 Compare December 30, 2019 19:15

wsfulton reviewed Dec 30, 2019

View reviewed changes

wsfulton mentioned this pull request Dec 30, 2019

Fix code generated for Ruby global variables #1668

Merged

treitmayr force-pushed the ruby2.7-support branch from 4105416 to ffb4a2b Compare December 31, 2019 16:48

treitmayr force-pushed the ruby2.7-support branch from ffb4a2b to 00e291b Compare December 31, 2019 16:54

Move new macros for Ruby to their dedicated namespace

5542cc2

wsfulton reviewed Jan 3, 2020

View reviewed changes

wsfulton added the Ruby label Jan 5, 2020

wsfulton merged commit f5908ec into swig:master Jan 5, 2020

chezou added a commit to chezou/Mykytea-ruby that referenced this pull request Mar 1, 2020

test: uncomment Ruby 2.7 on test

e8b21b2

Because swig 4.0.1 doesn't support Ruby 2.7 yet, stop testing Ruby 2.7. See also: swig/swig#1692

chezou mentioned this pull request Mar 1, 2020

Update SWIG generated file chezou/Mykytea-ruby#3

Merged

sodabrew mentioned this pull request Mar 1, 2020

Ruby 2.7 updates eventmachine/eventmachine#910

Merged

Conversation

treitmayr commented Dec 29, 2019

Uh oh!

anatol commented Dec 30, 2019

Uh oh!

treitmayr commented Dec 30, 2019

Uh oh!

treitmayr commented Dec 30, 2019

Uh oh!

wsfulton Dec 30, 2019

Choose a reason for hiding this comment

Uh oh!

treitmayr Dec 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wsfulton Jan 3, 2020

Choose a reason for hiding this comment

Uh oh!

treitmayr Jan 3, 2020

Choose a reason for hiding this comment

Uh oh!

treitmayr commented Dec 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wsfulton commented Jan 3, 2020

Uh oh!

treitmayr commented Jan 3, 2020

Uh oh!

wsfulton commented Jan 3, 2020

Uh oh!

wsfulton Jan 3, 2020

Choose a reason for hiding this comment

Uh oh!

treitmayr Jan 4, 2020

Choose a reason for hiding this comment

Uh oh!

treitmayr Jan 4, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

treitmayr Dec 30, 2019 •

edited

Loading

treitmayr commented Dec 31, 2019 •

edited

Loading