Bug 192743 – Use warning-ignoring macros more consistently and simply

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Darin Adler]

Use warning-ignoring macros more consistently and simply

[Darin Adler]

Created attachment 357405 [details]
Patch

[Mark Lam]

Comment on attachment 357405 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357405&action=review

r=me

> Source/WebKitLegacy/mac/WebCoreSupport/WebAlternativeTextClient.h:35
> +    explicit WebAlternativeTextClient(WebView *);

Remove space between "WebView" and "*".

> Source/WebKitLegacy/mac/WebCoreSupport/WebAlternativeTextClient.h:54
> +    WebView *m_webView;

Shouldn't this be "WebView* m_webView;"?  Why move the placement of the space?

[Darin Adler]

Comment on attachment 357405 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357405&action=review

>> Source/WebKitLegacy/mac/WebCoreSupport/WebAlternativeTextClient.h:35
>> +    explicit WebAlternativeTextClient(WebView *);
> 
> Remove space between "WebView" and "*".

In this case, WebView is an Objective-C class name. Our coding style document says that we should format it this way.

>> Source/WebKitLegacy/mac/WebCoreSupport/WebAlternativeTextClient.h:54
>> +    WebView *m_webView;
> 
> Shouldn't this be "WebView* m_webView;"?  Why move the placement of the space?

Ditto.

[Darin Adler]

Hmm, strangely the WebKit coding style guidelines on the website no longer say this. The now say “in non-C++ code” instead of having the rule depend on the type of the pointed-to object. I like the new guideline better, but I don’t remember the WebKit project discussion about making this change.

[Darin Adler]

I’ll make the changes you suggested, Mark. Not important either way, and I choose to interpret the wording in the latest draft of the document as a new guideline.

[Darin Adler]

Committed r239255: <https://trac.webkit.org/changeset/239255>

[Radar WebKit Bug Importer]

<rdar://problem/46757455>

[Alexey Proskuryakov]

(In reply to Darin Adler from comment #4)
> Hmm, strangely the WebKit coding style guidelines on the website no longer
> say this. The now say “in non-C++ code” instead of having the rule depend on
> the type of the pointed-to object. I like the new guideline better, but I
> don’t remember the WebKit project discussion about making this change.

Looks like the language in guidelines never changed. The rule for pointers was added in almost the current form in r12863 13 years ago. The only change is that references are no longer mentioned for non-C++ code.

But it is also my memory that we always interpreted this as object type difference, not as file type difference. Given that .m files can become .mm ones, having a fle type difference doesn't make a lot of sense.

[Michael Catanzaro]

(In reply to Alexey Proskuryakov from comment #8)
> But it is also my memory that we always interpreted this as object type
> difference, not as file type difference. Given that .m files can become .mm
> ones, having a fle type difference doesn't make a lot of sense.

FWIW it definitely makes sense for C vs. C++ types. E.g. we always write e.g. WebKitWebView* in WebKit C++ code rather than WebKitWebView * as we do in the public headers. Style checker would rightly throw a fit otherwise.

.m vs. .mm may be an entirely different matter and the desirable behavior there might be different, of course.