Skip to content

Commit 265ef5e

Browse files
kouthisisnic
kou
authored and
thisisnic
committed
GH-47205: [C++] Suppress GNU variadic macro warnings (#47286)
### Rationale for this change CRAN reports GNU variadic macro warnings. ### What changes are included in this PR? Don't use GNU variadic macro extension. ### Are these changes tested? Yes. ```bash archery docker run \ -e CC=clang \ -e CXX=clang++ \ -e Thrift_SOURCE=BUNDLED \ -e CXXFLAGS="-Wgnu-zero-variadic-macro-arguments -Wno-variadic-macro-arguments-omitted" \ fedora-cpp ``` ### Are there any user-facing changes? No. * GitHub Issue: #47205 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
1 parent aa8b20a commit 265ef5e

File tree

3 files changed

+191
-9
lines changed

3 files changed

+191
-9
lines changed

‎cpp/cmake_modules/ThirdpartyToolchain.cmake‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,8 +1738,29 @@ function(build_thrift)
17381738
if(CMAKE_VERSION VERSION_LESS 3.26)
17391739
message(FATAL_ERROR "Require CMake 3.26 or later for building bundled Apache Thrift")
17401740
endif()
1741+
set(THRIFT_PATCH_COMMAND)
1742+
if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
1743+
find_program(PATCH patch)
1744+
if(PATCH)
1745+
list(APPEND
1746+
THRIFT_PATCH_COMMAND
1747+
${PATCH}
1748+
-p1
1749+
-i)
1750+
else()
1751+
find_program(GIT git)
1752+
if(GIT)
1753+
list(APPEND THRIFT_PATCH_COMMAND ${GIT} apply)
1754+
endif()
1755+
endif()
1756+
if(THRIFT_PATCH_COMMAND)
1757+
# https://github.com/apache/thrift/pull/3187
1758+
list(APPEND THRIFT_PATCH_COMMAND ${CMAKE_CURRENT_LIST_DIR}/thrift-3187.patch)
1759+
endif()
1760+
endif()
17411761
fetchcontent_declare(thrift
17421762
${FC_DECLARE_COMMON_OPTIONS}
1763+
PATCH_COMMAND ${THRIFT_PATCH_COMMAND}
17431764
URL ${THRIFT_SOURCE_URL}
17441765
URL_HASH "SHA256=${ARROW_THRIFT_BUILD_SHA256_CHECKSUM}")
17451766

‎cpp/cmake_modules/thrift-3187.patch‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
From ad893633097b05ecdba8aa0f27aaf173dc7839b2 Mon Sep 17 00:00:00 2001
19+
From: Sutou Kouhei <[email protected]>
20+
Date: Fri, 8 Aug 2025 16:19:10 +0900
21+
Subject: [PATCH] THRIFT-3268: Suppress gnu-zero-variadic-macro-arguments
22+
warnings
23+
24+
Client: cpp
25+
26+
We can reproduce these warnings by:
27+
28+
CC=clang CXX=clang++ \
29+
cmake \
30+
-S . \
31+
-B ../thrift.build \
32+
-DWITH_{AS3,JAVA,JAVASCRIPT,NODEJS,PYTHON,C_GLIB}=OFF \
33+
-DCMAKE_CXX_FLAGS="-Wgnu-zero-variadic-macro-arguments"
34+
cmake --build ../thrift.build
35+
36+
Sample warning:
37+
38+
lib/cpp/src/thrift/TLogging.h:119:13: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
39+
119 | ##__VA_ARGS__); \
40+
| ^
41+
---
42+
lib/cpp/src/thrift/TLogging.h | 12 ++++++------
43+
lib/cpp/test/TransportTest.cpp | 12 ++++++------
44+
lib/cpp/test/ZlibTest.cpp | 6 ++----
45+
3 files changed, 14 insertions(+), 16 deletions(-)
46+
47+
diff --git a/lib/cpp/src/thrift/TLogging.h b/lib/cpp/src/thrift/TLogging.h
48+
index 07ff030f7da..64e9bf80bbb 100644
49+
--- a/lib/cpp/src/thrift/TLogging.h
50+
+++ b/lib/cpp/src/thrift/TLogging.h
51+
@@ -55,7 +55,7 @@
52+
#if T_GLOBAL_DEBUGGING_LEVEL > 0
53+
#define T_DEBUG(format_string, ...) \
54+
if (T_GLOBAL_DEBUGGING_LEVEL > 0) { \
55+
- fprintf(stderr, "[%s,%d] " format_string " \n", __FILE__, __LINE__, ##__VA_ARGS__); \
56+
+ fprintf(stderr, "[%s,%d] " format_string " \n", __FILE__, __LINE__, __VA_ARGS__); \
57+
}
58+
#else
59+
#define T_DEBUG(format_string, ...)
60+
@@ -80,7 +80,7 @@
61+
__FILE__, \
62+
__LINE__, \
63+
dbgtime, \
64+
- ##__VA_ARGS__); \
65+
+ __VA_ARGS__); \
66+
} \
67+
}
68+
#else
69+
@@ -96,7 +96,7 @@
70+
*/
71+
#define T_DEBUG_L(level, format_string, ...) \
72+
if ((level) > 0) { \
73+
- fprintf(stderr, "[%s,%d] " format_string " \n", __FILE__, __LINE__, ##__VA_ARGS__); \
74+
+ fprintf(stderr, "[%s,%d] " format_string " \n", __FILE__, __LINE__, __VA_ARGS__); \
75+
}
76+
77+
/**
78+
@@ -116,7 +116,7 @@
79+
__FILE__, \
80+
__LINE__, \
81+
dbgtime, \
82+
- ##__VA_ARGS__); \
83+
+ __VA_ARGS__); \
84+
}
85+
86+
/**
87+
@@ -137,7 +137,7 @@
88+
__FILE__, \
89+
__LINE__, \
90+
dbgtime, \
91+
- ##__VA_ARGS__); \
92+
+ __VA_ARGS__); \
93+
exit(1); \
94+
}
95+
96+
@@ -155,7 +155,7 @@
97+
time(&now); \
98+
THRIFT_CTIME_R(&now, dbgtime); \
99+
dbgtime[24] = '\0'; \
100+
- fprintf(stderr, "[%s] " format_string " \n", dbgtime, ##__VA_ARGS__); \
101+
+ fprintf(stderr, "[%s] " format_string " \n", dbgtime, __VA_ARGS__); \
102+
} \
103+
}
104+
#else
105+
diff --git a/lib/cpp/test/TransportTest.cpp b/lib/cpp/test/TransportTest.cpp
106+
index d6d38595a6b..8a05465773a 100644
107+
--- a/lib/cpp/test/TransportTest.cpp
108+
+++ b/lib/cpp/test/TransportTest.cpp
109+
@@ -784,23 +784,23 @@ void test_borrow_none_available() {
110+
**************************************************************************/
111+
112+
#define ADD_TEST_RW(CoupledTransports, totalSize, ...) \
113+
- addTestRW<CoupledTransports>(BOOST_STRINGIZE(CoupledTransports), totalSize, ##__VA_ARGS__);
114+
+ addTestRW<CoupledTransports>(BOOST_STRINGIZE(CoupledTransports), totalSize, __VA_ARGS__);
115+
116+
#define TEST_RW(CoupledTransports, totalSize, ...) \
117+
do { \
118+
/* Add the test as specified, to test the non-virtual function calls */ \
119+
- ADD_TEST_RW(CoupledTransports, totalSize, ##__VA_ARGS__); \
120+
+ ADD_TEST_RW(CoupledTransports, totalSize, __VA_ARGS__); \
121+
/* \
122+
* Also test using the transport as a TTransport*, to test \
123+
* the read_virt()/write_virt() calls \
124+
*/ \
125+
- ADD_TEST_RW(CoupledTTransports<CoupledTransports>, totalSize, ##__VA_ARGS__); \
126+
+ ADD_TEST_RW(CoupledTTransports<CoupledTransports>, totalSize, __VA_ARGS__); \
127+
/* Test wrapping the transport with TBufferedTransport */ \
128+
- ADD_TEST_RW(CoupledBufferedTransportsT<CoupledTransports>, totalSize, ##__VA_ARGS__); \
129+
+ ADD_TEST_RW(CoupledBufferedTransportsT<CoupledTransports>, totalSize, __VA_ARGS__); \
130+
/* Test wrapping the transport with TFramedTransports */ \
131+
- ADD_TEST_RW(CoupledFramedTransportsT<CoupledTransports>, totalSize, ##__VA_ARGS__); \
132+
+ ADD_TEST_RW(CoupledFramedTransportsT<CoupledTransports>, totalSize, __VA_ARGS__); \
133+
/* Test wrapping the transport with TZlibTransport */ \
134+
- ADD_TEST_RW(CoupledZlibTransportsT<CoupledTransports>, totalSize, ##__VA_ARGS__); \
135+
+ ADD_TEST_RW(CoupledZlibTransportsT<CoupledTransports>, totalSize, __VA_ARGS__); \
136+
} while (0)
137+
138+
#define ADD_TEST_BLOCKING(CoupledTransports) \
139+
diff --git a/lib/cpp/test/ZlibTest.cpp b/lib/cpp/test/ZlibTest.cpp
140+
index 274a243913c..ea9c617f625 100644
141+
--- a/lib/cpp/test/ZlibTest.cpp
142+
+++ b/lib/cpp/test/ZlibTest.cpp
143+
@@ -347,8 +347,7 @@ void test_get_underlying_transport() {
144+
do { \
145+
::std::ostringstream name_ss; \
146+
name_ss << name << "-" << BOOST_STRINGIZE(_FUNC); \
147+
- ::std::function<void ()> test_func = \
148+
- ::std::bind(_FUNC, ##__VA_ARGS__); \
149+
+ ::std::function<void ()> test_func = ::std::bind(_FUNC, __VA_ARGS__); \
150+
::boost::unit_test::test_case* tc \
151+
= ::boost::unit_test::make_test_case(test_func, name_ss.str(), __FILE__, __LINE__); \
152+
(suite)->add(tc); \
153+
@@ -359,8 +358,7 @@ void test_get_underlying_transport() {
154+
::std::ostringstream name_ss; \
155+
name_ss << name << "-" << BOOST_STRINGIZE(_FUNC); \
156+
::boost::unit_test::test_case* tc \
157+
- = ::boost::unit_test::make_test_case(::std::bind(_FUNC, \
158+
- ##__VA_ARGS__), \
159+
+ = ::boost::unit_test::make_test_case(::std::bind(_FUNC, __VA_ARGS__), \
160+
name_ss.str()); \
161+
(suite)->add(tc); \
162+
} while (0)

‎cpp/src/arrow/util/tracing_internal.h‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -149,14 +149,13 @@ opentelemetry::trace::StartSpanOptions SpanOptionsWithParent(
149149
target_span.details.get(), \
150150
::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__))))
151151

152-
# define START_SCOPED_SPAN_SV(target_span, name, ...) \
153-
::arrow::internal::tracing::Scope( \
154-
::arrow::internal::tracing::GetTracer()->WithActiveSpan( \
155-
::arrow::internal::tracing::RewrapSpan( \
156-
target_span.details.get(), \
157-
::arrow::internal::tracing::GetTracer()->StartSpan( \
158-
::opentelemetry::nostd::string_view(name.data(), name.size()), \
159-
##__VA_ARGS__))))
152+
# define START_SCOPED_SPAN_SV(target_span, name) \
153+
::arrow::internal::tracing::Scope( \
154+
::arrow::internal::tracing::GetTracer()->WithActiveSpan( \
155+
::arrow::internal::tracing::RewrapSpan( \
156+
target_span.details.get(), \
157+
::arrow::internal::tracing::GetTracer()->StartSpan( \
158+
::opentelemetry::nostd::string_view(name.data(), name.size())))))
160159

161160
# define START_SCOPED_SPAN_WITH_PARENT_SV(target_span, parent_span, name, ...) \
162161
::arrow::internal::tracing::Scope( \
@@ -226,7 +225,7 @@ struct Scope {
226225

227226
# define START_SPAN(target_span, ...)
228227
# define START_SCOPED_SPAN(target_span, ...) ::arrow::internal::tracing::Scope()
229-
# define START_SCOPED_SPAN_SV(target_span, name, ...) ::arrow::internal::tracing::Scope()
228+
# define START_SCOPED_SPAN_SV(target_span, name) ::arrow::internal::tracing::Scope()
230229
# define START_COMPUTE_SPAN(target_span, ...)
231230
# define ACTIVATE_SPAN(target_span) ::arrow::internal::tracing::Scope()
232231
# define MARK_SPAN(target_span, status)

0 commit comments

Comments
 (0)