...
Created attachment 375106 [details] proposed patch.
Created attachment 375107 [details] proposed patch.
Sign. This is more annoying to fix than I thought. It appears that GCC does not support ##__VA_ARGS__ expansion.
(In reply to Mark Lam from comment #3) > Sign. This is more annoying to fix than I thought. It appears that GCC > does not support ##__VA_ARGS__ expansion. Not true. I misread the build failure logs. Let me try again.
Oh thanks! (And nice timing, because this was coming up next on my TODO. :)
(In reply to Michael Catanzaro from comment #5) > Oh thanks! (And nice timing, because this was coming up next on my TODO. :) Can you take the bug? I don't have gcc and I can't reproduce the issue.
Yeah I'll try.
After reading https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html I was confused why your patch didn't work, but eventually I noticed: """ CPP retains the comma when conforming to a specific C standard. Otherwise the comma is dropped as an extension to the standard. """ Indeed, your patch works if I try this: diff --git a/Source/cmake/OptionsCommon.cmake b/Source/cmake/OptionsCommon.cmake index a574ef8c222..75f60ea9384 100644 --- a/Source/cmake/OptionsCommon.cmake +++ b/Source/cmake/OptionsCommon.cmake @@ -1,6 +1,6 @@ set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) -set(CMAKE_CXX_EXTENSIONS OFF) +set(CMAKE_CXX_EXTENSIONS ON) add_definitions(-DBUILDING_WITH_CMAKE=1) add_definitions(-DHAVE_CONFIG_H=1) Apple ports are compiled with GNU language extensions enabled (which clang implements for compatibility), but CMake ports are not. We could turn them on to make this work if Apple is OK with starting to use GNU extensions throughout the codebase, but it's probably better for WebKit to stick to language standards. The standard solution, according to the documentation, is to replace ##__VA_ARGS__ with format __VA_OPT__(,) __VA_ARGS__, but this requires C++20, and well, please no. Let me see if I can make it work by using the va_list in the function. It will probably mess up the stack and defeat the entire point of using the CRASH_WITH_INFO() macro, but this is just for the fallback path anyway.
(In reply to Michael Catanzaro from comment #8) > We could turn them > on to make this work if Apple is OK with starting to use GNU extensions > throughout the codebase (Currently they only work in Apple-specific code. That might be ironic.)
(In reply to Michael Catanzaro from comment #8) > Let me see if I can make it work by using the va_list in the function. Well that doesn't work because of course there's no way to get the vargs into the function without using __VA_ARGS__, oops.
OK, solution is just to use an inline function instead of a macro and call it a day.
Created attachment 375131 [details] Patch
Comment on attachment 375131 [details] Patch r=me if the EWS bots are happy.
Comment on attachment 375131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375131&action=review > Source/WTF/wtf/Assertions.h:606 > +// FIXME: When we enable C++20, we should replace ##__VA_ARGS with format __VA_OPT__(,) __VA_ARGS__ Typo here, should be ##__VA_ARGS__. Will land manually.
Committed r248008: <https://trac.webkit.org/changeset/248008>
<rdar://problem/53712058>