While compiling with GCC a lot of big warnings are shown because those macros boil down to vfprintf() in Linux, and that {public} specifier is invalid. It is only for os_log [1] 1. https://developer.apple.com/documentation/os/logging?language=objc
Created attachment 390262 [details] Patch
Comment on attachment 390262 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390262&action=review > Source/WTF/wtf/Assertions.h:487 > +#define LOGDSTR "s" In WebKit coding style we normally prefer words over clumps of letters. I don’t know that LOGDSTR is a good choice for the project. Is this referring to something named "logd"? Maybe PUBLIC_LOG_STRING would be a good name? Also, why not put the "%" into the macro?
(In reply to Darin Adler from comment #2) > Also, why not put the "%" into the macro? I think that leaving the "%" out of the macro follows standard macros like PRId64. Any naming convention should take into account other types of format specifications that might be used. For instance: PUBLIC_LOG_STRING: {public}s PRIVATE_LOG_STRING: {private}s FUTUREOPTION_LOG_STRING: {future-option}s PUBLIC_LOG_OBJCOBJECT: {public}@ PRIVATE_LOG_OBJCOBJECT: {private}@ FUTUREOPTION_LOG_OBJCOBJECT: {future-option}@ So while I like LOGDSTR for its brevity, it doesn't scale.
(In reply to Keith Rollin from comment #3) > (In reply to Darin Adler from comment #2) > > Also, why not put the "%" into the macro? > > I think that leaving the "%" out of the macro follows standard macros like > PRId64. It does. That alone doesn’t seem like reason to follow suit, though.
(In reply to Darin Adler from comment #4) > (In reply to Keith Rollin from comment #3) > > (In reply to Darin Adler from comment #2) > > > Also, why not put the "%" into the macro? > > > > I think that leaving the "%" out of the macro follows standard macros like > > PRId64. > > It does. That alone doesn’t seem like reason to follow suit, though. What's the reason for not following suit? Personally, I'd like to be consistent with the standard. That way I don't have to remember "OK, add the % for the standard macros, but leave it out for WebKit macros. Or is it the other way around...".
(In reply to Keith Rollin from comment #5) > (In reply to Darin Adler from comment #4) > > (In reply to Keith Rollin from comment #3) > > > (In reply to Darin Adler from comment #2) > > > > Also, why not put the "%" into the macro? > > > > > > I think that leaving the "%" out of the macro follows standard macros like > > > PRId64. > > > > It does. That alone doesn’t seem like reason to follow suit, though. > > What's the reason for not following suit? > > Personally, I'd like to be consistent with the standard. That way I don't > have to remember "OK, add the % for the standard macros, but leave it out > for WebKit macros. Or is it the other way around...". I do agree. It is better, IMO, to be consistent with other formatting macros.
Created attachment 390505 [details] Patch
(In reply to Keith Rollin from comment #3) > (In reply to Darin Adler from comment #2) > > Also, why not put the "%" into the macro? > > I think that leaving the "%" out of the macro follows standard macros like > PRId64. > > Any naming convention should take into account other types of format > specifications that might be used. For instance: > > PUBLIC_LOG_STRING: {public}s Done. Now this symbol is used. > PRIVATE_LOG_STRING: {private}s > FUTUREOPTION_LOG_STRING: {future-option}s > > PUBLIC_LOG_OBJCOBJECT: {public}@ > PRIVATE_LOG_OBJCOBJECT: {private}@ > FUTUREOPTION_LOG_OBJCOBJECT: {future-option}@ > > So while I like LOGDSTR for its brevity, it doesn't scale.
(In reply to Keith Rollin from comment #5) > What's the reason for not following suit? To me PUBLIC_LOG_STRING doesn’t seem much like PRId64. I agree that if you thought of them as similar and closely related then we’d want them to match. > Personally, I'd like to be consistent with the standard. That way I don't > have to remember "OK, add the % for the standard macros, but leave it out > for WebKit macros. Or is it the other way around...". OK, tastes differ, and I certainly won’t insist.
May I have r+ & q+ ? O:)
Comment on attachment 390505 [details] Patch Clearing flags on attachment: 390505 Committed r256745: <https://trac.webkit.org/changeset/256745>
All reviewed patches have been landed. Closing bug.
<rdar://problem/59517129>