1313903 - Fix -Wmismatched-parameter-types warning in widget/cocoa/nsAppShell.mm

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Chris Peterson [:cpeterson]]

Attachment: Wmismatched-parameter-types_nsAppShell.patch

I get the following clang warning after upgrading to Xcode 8.1:

widget/cocoa/nsAppShell.mm:113:47 [-Wmismatched-parameter-types] conflicting parameter types in implementation of 'nextEventMatchingMask:untilDate:inMode:dequeue:': 'NSEventMask' (aka 'unsigned long long') vs 'NSUInteger' (aka 'unsigned long')

NSWindow's nextEventMatchingMask method expects a parameter of type NSEventMask, not NSUInteger:

https://developer.apple.com/reference/appkit/nswindow/1419304-nexteventmatchingmask

Comment 1

[Chris Peterson [:cpeterson]]

Note that making this change (from NSUInteger to NSEventMask) introduces the reverse warning when compiling with older SDKs (used on the build machines). However, I still think we should move forward to NSEventMask, which is a more descriptive and future-proof type.

widget/cocoa/nsAppShell.mm:113:48: error: conflicting parameter types in implementation of 'nextEventMatchingMask:untilDate:inMode:dequeue:': 'NSUInteger' (aka 'unsigned long') vs 'NSEventMask' (aka 'unsigned long long') [-Werror,-Wmismatched-parameter-types]
- (NSEvent*)nextEventMatchingMask:(NSEventMask)mask

/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSApplication.h:202:48: note: previous definition is here
- (NSEvent *)nextEventMatchingMask:(NSUInteger)mask untilDate:(NSDate *)expiration inMode:(NSString *)mode dequeue:(BOOL)deqFlag;

Comment 2

[Stefan [:stefanh]]

It looks like they've switched parameter type in the 10.12 sdk (64-bit only). AFAICS all other sdk's uses NSUInteger.

Comment 3

[Stephen A Pohl [:spohl]]

Comment on attachment 8805826 [details] [diff] [review]
Wmismatched-parameter-types_nsAppShell.patch

Review of attachment 8805826 [details] [diff] [review]:
-----------------------------------------------------------------

Spoke with mstange and he agreed that ideally, we would support both the old and new type here.

::: widget/cocoa/nsAppShell.mm
@@ +109,5 @@
>    }
>    [super sendEvent:anEvent];
>  }
>  
> +- (NSEvent*)nextEventMatchingMask:(NSEventMask)mask

Could you #ifdef this such that we always use NSEventMask unless the SDK version is less than MAC_OS_X_VERSION_10_12, in which case we continue using NSUInteger?

Comment 4

[Chris Peterson [:cpeterson]]

Attachment: Wmismatched-parameter-types-v2.patch

(In reply to Stephen A Pohl [:spohl] from comment #3)
> Could you #ifdef this such that we always use NSEventMask unless the SDK
> version is less than MAC_OS_X_VERSION_10_12, in which case we continue using
> NSUInteger?

OK. That's a good suggestion. This patch adds #ifdefs for MAC_OS_X_VERSION_10_12 and __LP64__.

For posteriy, here is the #if __LP64__ used in 10.12's NSApplication.h:

#if __LP64__
- (nullable NSEvent *)nextEventMatchingMask:(NSEventMask)mask untilDate:(nullable NSDate *)expiration inMode:(NSRunLoopMode)mode dequeue:(BOOL)deqFlag;
…
#else
- (nullable NSEvent *)nextEventMatchingMask:(NSUInteger)mask untilDate:(nullable NSDate *)expiration inMode:(NSRunLoopMode)mode dequeue:(BOOL)deqFlag;
…
#endif

Comment 5

[Stephen A Pohl [:spohl]]

Comment on attachment 8806914 [details] [diff] [review]
Wmismatched-parameter-types-v2.patch

Review of attachment 8806914 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4148e7b3d684
Fix -Wmismatched-parameter-types warning in widget/cocoa/nsAppShell.mm. r=spohl

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/4148e7b3d684