Bug 195904

Summary: Make WTFLogChannelState and WTFLogLevel enum classes
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, commit-queue, eric.carlson, ews-watchlist, mcatanzaro, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

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 EWS Watchlist 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 EWS Watchlist 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 EWS Watchlist 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.