Bug 126055 – #defined constants should use all uppercase names with words separated by underscores.

• Status: REOPENED
• Component: Tools / Tests

[Gergő Balogh]

wrong:
#define lowercase

good:
#define UPPER_CASE

[Gergő Balogh]

Created attachment 220418 [details]
patch

[Alexey Proskuryakov]

Comment on attachment 220418 [details]
patch

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

> Tools/Scripts/webkitpy/style/checkers/cpp.py:2726
> +        if not match(r'^[A-Z_]+$', name):

This expression doesn't allow for digits, which may be legitimately present in macro names.

[Gergő Balogh]

Created attachment 220500 [details]
patch fix

I just fixed the above mentioned regex error.

[WebKit Commit Bot]

Comment on attachment 220500 [details]
patch fix

Clearing flags on attachment: 220500

Committed r161427: <http://trac.webkit.org/changeset/161427>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 126645

[Alexey Proskuryakov]

Turns out that we have macros that intentionally do not follow these rules - all include guards in headers are like this. Rolling out, because this style error is now emitted for all newly added header files.

See bug 126626 comment 4 for an example.

[Darin Adler]

Header guards are macros that are not made of all capital letters. However, they are distinctive because they end “_h”, so we could possibly refine the rule to understand that.

We still should run the script across the entire source tree before checking it in to make sure that all the things it complains about are actual problems and there are no false positives.

[Gergő Balogh]

Created attachment 221952 [details]
Patch

[Darin Adler]

What were the results when you ran this across the entire source tree? Did you find any other obvious false positives? Could you post the results file from running on the current source tree?

[Michael Catanzaro]

Comment on attachment 221952 [details]
Patch

Hi,

Apologies that your patch was not reviewed in a timely manner. Since it's now quite old, I am removing it from the review request queue. Please consider rebasing it on trunk and resubmitting.

To increase the chances of getting a review, consider using 'Tools/Scripts/webkit-patch upload --suggest-reviewers' to CC reviewers who might be interested in this bug.