Bug 107360

Summary: Remove unnecessary PLATFORM() tests
Product: WebKit Reporter: Laszlo Gombos <laszlo.gombos>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, darin, eric, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed change none

Description Laszlo Gombos 2013-01-18 22:56:05 PST 
PLATFORM(WIN) PLATFORM(CHROMIUM) and PLATFORM(QT) are mutually exclusive - only one of them can be true. In a few places we have tests like "PLATFORM(WIN) && !PLATFORM(CHROMIUM) && !PLATFORM(QT)". This is probably a left over from the times when PLATFORM(WIN) use to be OS(WIN).
Comment 1 Laszlo Gombos 2013-01-18 22:59:54 PST 
Created attachment 183605 [details]
proposed change
Comment 2 Eric Seidel (no email) 2013-01-18 23:01:43 PST 
Comment on attachment 183605 [details]
proposed change

Because PLATFORM(WIN) means AppleWin?
Comment 3 Laszlo Gombos 2013-01-18 23:08:10 PST 
(In reply to comment #2)
> (From update of attachment 183605 [details])
> Because PLATFORM(WIN) means AppleWin?

Yes, indeed.
Comment 4 Laszlo Gombos 2013-01-18 23:09:49 PST 
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 183605 [details] [details])
> > Because PLATFORM(WIN) means AppleWin?
> 
> Yes, indeed.

These PLATFORM() macros are setup in Platform.h and from the definition it is clear that there are mutually exclusive.


#if defined(BUILDING_CHROMIUM__)
#define WTF_PLATFORM_CHROMIUM 1 
#elif defined(BUILDING_QT__)    
#define WTF_PLATFORM_QT 1       
#elif defined(BUILDING_WX__)    
#define WTF_PLATFORM_WX 1       
#elif defined(BUILDING_EFL__)   
#define WTF_PLATFORM_EFL 1      
#elif defined(BUILDING_GTK__)   
#define WTF_PLATFORM_GTK 1      
#elif defined(BUILDING_BLACKBERRY__)
#define WTF_PLATFORM_BLACKBERRY 1   
#elif OS(DARWIN)
#define WTF_PLATFORM_MAC 1
#elif OS(WINDOWS)
#define WTF_PLATFORM_WIN 1
#endif
Comment 5 Eric Seidel (no email) 2013-01-18 23:15:55 PST 
Comment on attachment 183605 [details]
proposed change

OK.  One of these years we should rename PLATFORM(MAC) and PLATFORM(WIN) to PLATFORM(APPLE_MAC) and APPLE_WIN to end the confusion.
Comment 6 WebKit Review Bot 2013-01-18 23:50:10 PST 
Comment on attachment 183605 [details]
proposed change

Clearing flags on attachment: 183605

Committed r140238: <http://trac.webkit.org/changeset/140238>
Comment 7 WebKit Review Bot 2013-01-18 23:50:14 PST 
All reviewed patches have been landed.  Closing bug.
Comment 8 Darin Adler 2013-01-19 08:30:30 PST 
(In reply to comment #5)
> OK.  One of these years we should rename PLATFORM(MAC) and PLATFORM(WIN) to PLATFORM(APPLE_MAC) and APPLE_WIN to end the confusion.

Renaming is fine, but even though they seem different to you, I don’t think that most people would understand APPLE_MAC as distinct from MAC, so I think we have to think more on how to name.