THRIFT-3268: Suppress gnu-zero-variadic-macro-arguments warnings #3187
+14
−16
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Client: cpp
We can reproduce these warnings by:
CC=clang CXX=clang++ \
cmake \
-S . \
-B ../thrift.build \
-DWITH_{AS3,JAVA,JAVASCRIPT,NODEJS,PYTHON,C_GLIB}=OFF \
-DCMAKE_CXX_FLAGS="-Wgnu-zero-variadic-macro-arguments"
cmake --build ../thrift.build
Sample warning:
lib/cpp/src/thrift/TLogging.h:119:13: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
119 | ##__VA_ARGS__); \
| ^
Member
|
I'm not so sure about this one. Introducing a breaking change (per JIRA label) and looking at the considerations behind (e.g. this SO thread to name just one) that looks to me like a simple but effective way to introduce funny issues. |
Member
Author
|
OK. Are you interested in suppressing this warning? If so, I'll look into another approach that doesn't introduce any backward compatibility. If you aren't interested in suppressing this warning, I close this. (Should we close https://issues.apache.org/jira/browse/THRIFT-3268 too?) |
Member
|
My question was merely how can we make this non-breaking? |
Member
Author
|
OK. How about using diff --git a/lib/cpp/src/thrift/TLogging.h b/lib/cpp/src/thrift/TLogging.h
index 07ff030f7..c4935f757 100644
--- a/lib/cpp/src/thrift/TLogging.h
+++ b/lib/cpp/src/thrift/TLogging.h
@@ -53,9 +53,11 @@
* @param format_string
*/
#if T_GLOBAL_DEBUGGING_LEVEL > 0
-#define T_DEBUG(format_string, ...) \
+#define T_DEBUG(...) \
if (T_GLOBAL_DEBUGGING_LEVEL > 0) { \
- fprintf(stderr, "[%s,%d] " format_string " \n", __FILE__, __LINE__, ##__VA_ARGS__); \
+ fprintf(stderr, "[%s,%d] ", __FILE__, __LINE__); \
+ fprintf(stderr, __VA_ARGS__); \
+ fprintf(stderr, " \n"); \
}
#else
#define T_DEBUG(format_string, ...)
@@ -67,7 +69,7 @@
* @param string format_string input: printf style format string
*/
#if T_GLOBAL_DEBUGGING_LEVEL > 0
-#define T_DEBUG_T(format_string, ...) \
+#define T_DEBUG_T(...) \
{ \
if (T_GLOBAL_DEBUGGING_LEVEL > 0) { \
time_t now; \
@@ -76,11 +78,12 @@
THRIFT_CTIME_R(&now, dbgtime); \
dbgtime[24] = '\0'; \
fprintf(stderr, \
- "[%s,%d] [%s] " format_string " \n", \
+ "[%s,%d] [%s] ", \
__FILE__, \
__LINE__, \
- dbgtime, \
- ##__VA_ARGS__); \
+ dbgtime); \
+ fprintf(stderr, __VA_ARGS__); \
+ fprintf(stderr, " \n"); \
} \
}
#else
@@ -94,9 +97,11 @@
* @param int level: specified debug level
* @param string format_string input: format string
*/
-#define T_DEBUG_L(level, format_string, ...) \
+#define T_DEBUG_L(level, ...) \
if ((level) > 0) { \
- fprintf(stderr, "[%s,%d] " format_string " \n", __FILE__, __LINE__, ##__VA_ARGS__); \
+ fprintf(stderr, "[%s,%d] ", __FILE__, __LINE__); \
+ fprintf(stderr, __VA_ARGS__); \
+ fprintf(stderr, " \n"); \
}
/**
@@ -104,7 +109,7 @@
*
* @param string format_string input: printf style format string
*/
-#define T_ERROR(format_string, ...) \
+#define T_ERROR(...) \
{ \
time_t now; \
char dbgtime[26]; \
@@ -112,11 +117,12 @@
THRIFT_CTIME_R(&now, dbgtime); \
dbgtime[24] = '\0'; \
fprintf(stderr, \
- "[%s,%d] [%s] ERROR: " format_string " \n", \
+ "[%s,%d] [%s] ERROR: ", \
__FILE__, \
__LINE__, \
- dbgtime, \
- ##__VA_ARGS__); \
+ dbgtime); \
+ fprintf(stderr, __VA_ARGS__); \
+ fprintf(stderr, " \n"); \
}
/**
@@ -125,7 +131,7 @@
*
* @param string format_string input: printf style format string
*/
-#define T_ERROR_ABORT(format_string, ...) \
+#define T_ERROR_ABORT(...) \
{ \
time_t now; \
char dbgtime[26]; \
@@ -133,11 +139,12 @@
THRIFT_CTIME_R(&now, dbgtime); \
dbgtime[24] = '\0'; \
fprintf(stderr, \
- "[%s,%d] [%s] ERROR: Going to abort " format_string " \n", \
+ "[%s,%d] [%s] ERROR: Going to abort ", \
__FILE__, \
__LINE__, \
- dbgtime, \
- ##__VA_ARGS__); \
+ dbgtime); \
+ fprintf(stderr, __VA_ARGS__); \
+ fprintf(stderr, " \n"); \
exit(1); \
}
@@ -147,7 +154,7 @@
* @param string format_string input: printf style format string
*/
#if T_GLOBAL_LOGGING_LEVEL > 0
-#define T_LOG_OPER(format_string, ...) \
+#define T_LOG_OPER(...) \
{ \
if (T_GLOBAL_LOGGING_LEVEL > 0) { \
time_t now; \
@@ -155,7 +162,9 @@
time(&now); \
THRIFT_CTIME_R(&now, dbgtime); \
dbgtime[24] = '\0'; \
- fprintf(stderr, "[%s] " format_string " \n", dbgtime, ##__VA_ARGS__); \
+ fprintf(stderr, "[%s] ", dbgtime); \
+ fprintf(stderr, __VA_ARGS__); \
+ fprintf(stderr, " \n"); \
} \
}
#else |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Client: cpp
We can reproduce these warnings by:
Sample warning:
T_DEBUG("...")is rejected by this change. We need to useT_DEBUG("%s", "...")instead.[skip ci]anywhere in the commit message to free up build resources.