Project

General

Profile

Actions

Bug #17865

closed

clang 12 -Wcompound-token-split-by-macro warning in ruby.h

Added by _dim (Dimitry Andric) almost 3 years ago. Updated over 2 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 2.7.3p183 (2021-04-05 revision 6847ee089d) [amd64-freebsd14]
[ruby-core:103861]

Description

As reported in pull request #4504 (originally via FreeBSD PR 255910), 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:

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-project@0e00a95).

In the pull request I have submitted a possible fix for ruby 2.7 (and 2.6), because the above warnings do not occur on 3.0 and master: in GitHub commit 6ecf07ab35 (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 commit 9e6e39c351 this got merged into master.

Actions #1

Updated by nobu (Nobuyoshi Nakada) almost 3 years ago

  • Backport changed from 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN to 2.6: REQUIRED, 2.7: REQUIRED, 3.0: DONTNEED

Updated by nobu (Nobuyoshi Nakada) almost 3 years ago

  • Status changed from Open to Closed

Backport-ready.

Updated by Eregon (Benoit Daloze) over 2 years ago

This looks like it was not yet backported to 2.7: https://github.com/ruby/ruby/blob/ruby_2_7/include/ruby/ruby.h#L1847
However it seems the PR is ready: https://github.com/ruby/ruby/pull/4504
@nobu (Nobuyoshi Nakada) Could you review/approve that PR?
@nagachika (Tomoyuki Chikanaga) Could you integrate it?

Updated by nagachika (Tomoyuki Chikanaga) over 2 years ago

The PR looks ok to me. The current ruby_2_7 branch maintainer is usa-san.

Updated by usa (Usaku NAKAMURA) over 2 years ago

  • Backport changed from 2.6: REQUIRED, 2.7: REQUIRED, 3.0: DONTNEED to 2.6: REQUIRED, 2.7: DONE, 3.0: DONTNEED

backported into ruby_2_7 via PR. thx!

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0