Bug 195904 - Make WTFLogChannelState and WTFLogLevel enum classes
Summary: Make WTFLogChannelState and WTFLogLevel enum classes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-18 13:06 PDT by Alex Christensen
Modified: 2019-03-19 09:47 PDT (History)
6 users (show)

See Also:


Attachments
Patch (57.36 KB, patch)
2019-03-18 13:09 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (44.54 KB, patch)
2019-03-18 14:38 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (46.30 KB, patch)
2019-03-18 15:24 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (46.26 KB, patch)
2019-03-19 09:06 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2019-03-18 13:06:15 PDT
Make WTFLogChannelState and WTFLogLevel enum classes
Comment 1 Alex Christensen 2019-03-18 13:09:32 PDT
Created attachment 365047 [details]
Patch
Comment 2 Eric Carlson 2019-03-18 13:31:06 PDT
Comment on attachment 365047 [details]
Patch

Thanks Alex!
Comment 3 Alex Christensen 2019-03-18 14:38:00 PDT
Created attachment 365067 [details]
Patch
Comment 4 Build Bot 2019-03-18 14:40:28 PDT
Attachment 365067 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Assertions.h:182:  LOG_CHANNEL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Assertions.h:185:  LOG_CHANNEL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Alex Christensen 2019-03-18 15:24:27 PDT
Created attachment 365075 [details]
Patch
Comment 6 Build Bot 2019-03-18 15:29:15 PDT
Attachment 365075 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Assertions.h:182:  LOG_CHANNEL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Assertions.h:185:  LOG_CHANNEL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 WebKit Commit Bot 2019-03-18 23:36:17 PDT
Comment on attachment 365075 [details]
Patch

Clearing flags on attachment: 365075

Committed r243132: <https://trac.webkit.org/changeset/243132>
Comment 8 WebKit Commit Bot 2019-03-18 23:36:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2019-03-18 23:37:42 PDT
<rdar://problem/49008895>
Comment 10 Michael Catanzaro 2019-03-19 06:21:14 PDT
GTK is broken:

In file included from /usr/include/X11/Xlib.h:44,
                 from ../../Source/WebCore/plugins/npapi.h:89,
                 from ../../Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npapi.h:1,
                 from ../../Tools/DumpRenderTree/TestNetscapePlugIn/TestObject.h:26,
                 from ../../Tools/DumpRenderTree/TestNetscapePlugIn/TestObject.cpp:26:
../../Source/WTF/wtf/Assertions.h:153:36: error: expected identifier before numeric constant
  153 | enum class WTFLogLevel : uint8_t { Always, Error, Warning, Info, Debug };
      |                                    ^~~~~~

We won't ever be able to use the name "Always" because it makes Assertions.h conflict with Xlib.h, and it's not practical to ensure these headers are never included together. Xlib.h is the absolute worst (it defines a bunch of very common names without any namespace) but we're stuck with it.
Comment 11 Michael Catanzaro 2019-03-19 06:24:57 PDT
I think the options are:

 (a) Rename WTFLogLevel::Always to something less-appropriate
 (b) Remain an old-style enum forever

Let's go with (b) for now, just to get the build working until we have time to bikeshed about naming.
Comment 12 Michael Catanzaro 2019-03-19 06:25:21 PDT
Reverted r243132 for reason:

Broke GTK build

Committed r243135: <https://trac.webkit.org/changeset/243135>
Comment 13 Alex Christensen 2019-03-19 07:03:29 PDT
WTFLogLevel::Always_ could work
Comment 14 Michael Catanzaro 2019-03-19 07:59:10 PDT
(In reply to Michael Catanzaro from comment #11)
> I think the options are:
> 
>  (a) Rename WTFLogLevel::Always to something less-appropriate
>  (b) Remain an old-style enum forever
> 
> Let's go with (b) for now, just to get the build working until we have time
> to bikeshed about naming.

Phil suggested #undef Always, so that's another option. It will fail if Assertion.h gets included before anything that actually needs to use Xlib.h's Always. I think nothing in WebKit will ever want to do that, but other external headers might... using #undef in headers is not exactly safe. Then again, nothing about unified builds is safe.
Comment 15 Alex Christensen 2019-03-19 09:06:57 PDT
Created attachment 365168 [details]
Patch
Comment 16 Build Bot 2019-03-19 09:09:25 PDT
Attachment 365168 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Assertions.h:183:  LOG_CHANNEL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Assertions.h:186:  LOG_CHANNEL is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 WebKit Commit Bot 2019-03-19 09:47:10 PDT
Comment on attachment 365168 [details]
Patch

Clearing flags on attachment: 365168

Committed r243141: <https://trac.webkit.org/changeset/243141>
Comment 18 WebKit Commit Bot 2019-03-19 09:47:11 PDT
All reviewed patches have been landed.  Closing bug.