Bug 126055

Summary:

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

Product:

WebKit

Reporter:

Gergő Balogh <gbalogh.u-szeged>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

REOPENED

Severity:

Normal

CC:

ap, commit-queue, darin, glenn

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

126645

Bug Blocks:

Attachments:

Description	Flags
patch	ap: review-, ap: commit-queue-
patch fix	none
Patch	none

Gergő Balogh

Reported 2013-12-19 23:28:03 PST

wrong: #define lowercase good: #define UPPER_CASE

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Gergő Balogh

Comment 1 2014-01-06 00:52:11 PST

Created attachment 220418 [details] patch

Alexey Proskuryakov

Comment 2 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.

Gergő Balogh

Comment 3 2014-01-07 01:15:22 PST

Created attachment 220500 [details] patch fix I just fixed the above mentioned regex error.

WebKit Commit Bot

Comment 4 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>

WebKit Commit Bot

Comment 5 2014-01-07 08:55:21 PST

All reviewed patches have been landed. Closing bug.

WebKit Commit Bot

Comment 6 2014-01-08 09:41:56 PST

Re-opened since this is blocked by bug 126645

Alexey Proskuryakov

Comment 7 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.

Darin Adler

Comment 8 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.

Gergő Balogh

Comment 9 2014-01-22 23:57:57 PST

Created attachment 221952 [details] Patch

Darin Adler

Comment 10 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?

Michael Catanzaro

Comment 11 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.

Note You need to log in before you can comment on or make changes to this bug.