Bug 126055 - #defined constants should use all uppercase names with words separated by underscores.
Summary: #defined constants should use all uppercase names with words separated by und...
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 126645
Blocks:
  Show dependency treegraph
 
Reported: 2013-12-19 23:28 PST by Gergő Balogh
Modified: 2016-09-17 07:19 PDT (History)
4 users (show)

See Also:


Attachments
patch (3.36 KB, patch)
2014-01-06 00:52 PST, Gergő Balogh
ap: review-
ap: commit-queue-
Details | Formatted Diff | Diff
patch fix (3.37 KB, patch)
2014-01-07 01:15 PST, Gergő Balogh
no flags Details | Formatted Diff | Diff
Patch (3.70 KB, patch)
2014-01-22 23:57 PST, Gergő Balogh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gergő Balogh 2013-12-19 23:28:03 PST
wrong:
#define lowercase

good:
#define UPPER_CASE
Comment 1 Gergő Balogh 2014-01-06 00:52:11 PST
Created attachment 220418 [details]
patch
Comment 2 Alexey Proskuryakov 2014-01-06 09:49:38 PST
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.
Comment 3 Gergő Balogh 2014-01-07 01:15:22 PST
Created attachment 220500 [details]
patch fix

I just fixed the above mentioned regex error.
Comment 4 WebKit Commit Bot 2014-01-07 08:55:19 PST
Comment on attachment 220500 [details]
patch fix

Clearing flags on attachment: 220500

Committed r161427: <http://trac.webkit.org/changeset/161427>
Comment 5 WebKit Commit Bot 2014-01-07 08:55:21 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 WebKit Commit Bot 2014-01-08 09:41:56 PST
Re-opened since this is blocked by bug 126645
Comment 7 Alexey Proskuryakov 2014-01-08 09:42:50 PST
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.
Comment 8 Darin Adler 2014-01-08 17:27:59 PST
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.
Comment 9 Gergő Balogh 2014-01-22 23:57:57 PST
Created attachment 221952 [details]
Patch
Comment 10 Darin Adler 2014-01-23 07:23:02 PST
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?
Comment 11 Michael Catanzaro 2016-09-17 07:19:51 PDT
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.