Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ruby] Fix compilation errors on clang 16 #34481

Closed
wants to merge 1 commit into from

Conversation

stanhu
Copy link
Contributor

@stanhu stanhu commented Sep 26, 2023

As announced in https://discourse.llvm.org/t/clang-16-notice-of-potentially-breaking-changes/65562, clang 16 defaults -Wincompatible-function-pointer-types to be on. This causes a number of hard errors due to implicit conversions between void * and void, such as:

../../../../src/ruby/ext/grpc/rb_channel.c:770:47: error: incompatible function pointer types passing 'VALUE (VALUE)' (aka 'unsigned long (unsigned long)') to parameter of type 'VALUE (*)(void *)' (aka 'unsigned long (*)(void *)') [-Wincompatible-function-pointer-types]
  g_channel_polling_thread = rb_thread_create(run_poll_channels_loop, NULL);
                                              ^~~~~~~~~~~~~~~~~~~~~~
/root/.rbenv/versions/3.1.4/include/ruby-3.1.0/ruby/internal/intern/thread.h:190:32: note: passing argument to parameter 'f' here
VALUE rb_thread_create(VALUE (*f)(void *g), void *g);
                               ^
../../../../src/ruby/ext/grpc/rb_channel.c:780:41: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
void grpc_rb_channel_polling_thread_stop() {
                                        ^
                                         void
../../../../src/ruby/ext/grpc/rb_channel.c:786:30: error: incompatible function pointer types passing 'void (void *)' to parameter of type 'void *(*)(void *)' [-Wincompatible-function-pointer-types]
  rb_thread_call_without_gvl(run_poll_channels_loop_unblocking_func, NULL, NULL,

This commit fixes these pointer types using wrapper functions where necessary.

This issue was also raised on the FreeBSD port of the grpc gem: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271540

@stanhu
Copy link
Contributor Author

stanhu commented Sep 26, 2023

@alto-ruby @veblush Homebrew users of clang 16 found that grpc fails to build without this change. I see that #32920 adds CI support for clang 16.

@alto-ruby
Copy link
Contributor

LGTM.
@apolcyn could you take a look as well?
@stanhu thanks for the PR, could you rebase to the latest master?

@alto-ruby
Copy link
Contributor

alto-ruby commented Dec 12, 2023

Sanity checks failed with formatting errors: https://source.cloud.google.com/results/invocations/8143196f-0e53-4d33-a1a6-63577c48f97a/targets/github%2Fgrpc%2Frun_tests%2Fsanity_linux_dbg_native%2Ftools%2Fdistrib%2Fclang_format_code.sh/tests

If you have trouble running the clangformat script, format in vscode would works well.

As announced in
https://discourse.llvm.org/t/clang-16-notice-of-potentially-breaking-changes/65562,
clang 16 defaults `-Wincompatible-function-pointer-types` to be
on. This causes a number of hard errors due to implicit conversions
between `void *` and `void`, such as:

```
../../../../src/ruby/ext/grpc/rb_channel.c:770:47: error: incompatible function pointer types passing 'VALUE (VALUE)' (aka 'unsigned long (unsigned long)') to parameter of type 'VALUE (*)(void *)' (aka 'unsigned long (*)(void *)') [-Wincompatible-function-pointer-types]
  g_channel_polling_thread = rb_thread_create(run_poll_channels_loop, NULL);
                                              ^~~~~~~~~~~~~~~~~~~~~~
/root/.rbenv/versions/3.1.4/include/ruby-3.1.0/ruby/internal/intern/thread.h:190:32: note: passing argument to parameter 'f' here
VALUE rb_thread_create(VALUE (*f)(void *g), void *g);
                               ^
../../../../src/ruby/ext/grpc/rb_channel.c:780:41: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
void grpc_rb_channel_polling_thread_stop() {
                                        ^
                                         void
../../../../src/ruby/ext/grpc/rb_channel.c:786:30: error: incompatible function pointer types passing 'void (void *)' to parameter of type 'void *(*)(void *)' [-Wincompatible-function-pointer-types]
  rb_thread_call_without_gvl(run_poll_channels_loop_unblocking_func, NULL, NULL,
```

This commit fixes these pointer types using wrapper functions where
necessary.

This issue was also raised on the FreeBSD port of the grpc gem:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271540
@veblush veblush added release notes: no Indicates if PR should not be in release notes kokoro:run labels Dec 13, 2023
@alto-ruby
Copy link
Contributor

alto-ruby commented Jan 11, 2024

@apolcyn Hi Alex do you have comments? Could we get your approval of this PR?
I'll wait one more week to see if you have any concerns, if not, I'll go ahead and merge it. Sounds OK?

@HannahShiSFB
Copy link
Collaborator

@sampajano Hi Eryu, could you help me check the "import/copybara Pending" status? Thanks a lot!

@sampajano
Copy link
Contributor

@HannahShiSFB Thanks for checking! I've approved the internal merge request and will find one more reviewer to approve too (2 is required)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kokoro:run lang/ruby release notes: no Indicates if PR should not be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants