Fix clang 12 -Wcompound-token-split-by-macro warning in ruby.h #4504
Conversation
Building certain ruby gem native extensions (such as thrift), with clang
12.0.0 or later fails, because they have -Werror in their CFLAGS,
resulting in complaints about the expansion of the `rb_intern()` macro:
```
current directory: /wrkdirs/usr/ports/devel/rubygem-thrift/work/stage/usr/local/lib/ruby/gems/2.7/gems/thrift-0.14.0/ext
make "DESTDIR="
compiling binary_protocol_accelerated.c
binary_protocol_accelerated.c:404:68: error: '(' and '{' tokens introducing statement expression appear in different macro expansion contexts [-Werror,-Wcompound-token-split-by-macro]
VALUE thrift_binary_protocol_class = rb_const_get(thrift_module, rb_intern("BinaryProtocol"));
^~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/ruby-2.7/ruby/ruby.h:1847:23: note: expanded from macro 'rb_intern'
__extension__ (RUBY_CONST_ID_CACHE((ID), (str))) : \
^
binary_protocol_accelerated.c:404:68: note: '{' token is here
VALUE thrift_binary_protocol_class = rb_const_get(thrift_module, rb_intern("BinaryProtocol"));
^~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/ruby-2.7/ruby/ruby.h:1847:24: note: expanded from macro 'rb_intern'
__extension__ (RUBY_CONST_ID_CACHE((ID), (str))) : \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/ruby-2.7/ruby/ruby.h:1832:5: note: expanded from macro 'RUBY_CONST_ID_CACHE'
{ \
^
```
Part of the `rb_intern()` macro expands to `(RUBY_CONST_ID_CACHE((ID),
(str)))`, and in turn `RUBY_CONST_ID_CACHE()` expands to a brace
enclosed compound statement. The intended effect is to get a gcc
statement expression, which is normally delimited by `({ ... })`.
However, clang 12.0.0 and later have a warning enabled by default, about
pasting together the `(` and `{` tokens via different macros (see
<llvm/llvm-project@0e00a95>).
To work around this warning:
* Add `RUBY_CONST_ID_CACHE_NB()` (i.e. no-brace) which contains the code
itself, without any braces
* `RUBY_CONST_ID_CACHE()` which uses `RUBY_CONST_ID_CACHE_NB()`, but
puts braces around it (so no existing code using this macro breaks)
* Finally, change `rb_intern()` so the `__extension__` directly creates
a gcc statement expression, using the `RUBY_CONST_ID_CACHE_NB()` macro
|
Note, I'm going to submit similar patches for the FreeBSD ruby 2.6 and 2.7 ports. If this approach is OK with you, I will also submit a similar pull request for the ruby_2_6 branch. Also, this fix does not apply to ruby 3.0, as the main ruby.h header has been split up into parts, and the |
|
To backport commits from |
Unfortunately this is not possible, because the code in master does not have the issue, it is very different. E.g. in 6ecf07a (for pull request #2991) the symbol.h header got split off from the main ruby.h, and at the same time the code that emits the warning was removed. Later in 9e6e39c this got merged into master. I will close this pull request, and we will solve the problems in FreeBSD ports with custom patches instead. |
|
I see, so the PR itself is fine. |
I submitted https://bugs.ruby-lang.org/issues/17865, but I am not able to edit the backports field. (I assume this is limited to administrators.) |
|
@DimitryAndric Are you aware of any work arounds for end users here? I'm blocked by being unable to install the Thrift gem |
Well, the easiest workaround is to manipulate your should work? Although I'm unsure if that will append to, or completely replace, the |
This didn't quite work for me, but |
Building certain ruby gem native extensions (such as thrift, see here and here), with clang 12.0.0 or later fails, because they have -Werror in their CFLAGS, resulting in complaints about the expansion of the
rb_intern()macro:Part of the
rb_intern()macro expands to(RUBY_CONST_ID_CACHE((ID), (str))), and in turnRUBY_CONST_ID_CACHE()expands to a brace enclosed compound statement. The intended effect is to get a gcc statement expression, which is normally delimited by({ ... }).However, clang 12.0.0 and later have a warning enabled by default, about pasting together the
(and{tokens via different macros (see llvm/llvm-project@0e00a95).To work around this warning:
RUBY_CONST_ID_CACHE_NB()(i.e. no-brace) which contains the code itself, without any bracesRUBY_CONST_ID_CACHE()which usesRUBY_CONST_ID_CACHE_NB(), but puts braces around it (so no existing code using this macro breaks)rb_intern()so the__extension__directly creates a gcc statement expression, using theRUBY_CONST_ID_CACHE_NB()macro