1296967 - widget/cocoa/nsCocoaWindow.mm:519:3: warning: ignoring return value of function declared with warn_unused_result attribute [-Wunused-result]

Status: RESOLVED FIXED

Description

[Chris Peterson [:cpeterson]]

Created attachment 8783345 [details] [diff] [review]
Wunused-result_nsCocoaWindow.patch

Bug 1293596 added MOZ_MUST_USE annotations to the return value of  nsIWidget::Create() and derived classes' Create() overrides:

https://hg.mozilla.org/integration/mozilla-inbound/rev/a3f8c906f4022dfd3a5f35aa1e08178660a8c95d

widget/cocoa/nsCocoaWindow.mm:519:3: warning: ignoring return value of function declared with warn_unused_result attribute [-Wunused-result]
mPopupContentView->Create(thisAsWidget, nullptr, aRect, nullptr);

Comment 1

[Stephen A Pohl [:spohl]]

Comment on attachment 8783345 [details] [diff] [review]
Wunused-result_nsCocoaWindow.patch

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

r+ with nit addressed. Thank you!

::: widget/cocoa/nsCocoaWindow.mm
@@ +515,5 @@
>  
>    NS_ADDREF(mPopupContentView);
>  
>    nsIWidget* thisAsWidget = static_cast<nsIWidget*>(this);
> +  nsresult rv = mPopupContentView->Create(thisAsWidget, nullptr, aRect, nullptr);

nit: this line should be broken up to fit within the 80 character per line limit. It's currently at 82.

Side-note: Yes, there are many others in this file but we're trying to get new code to abide by the rule. :-)

Comment 2

[Nicholas Nethercote [:njn]]

I was wondering how this happened, and I see that widget/cocoa/ is one of only two directories in the entire tree (the other being config/external/nss) where we still have this in the moz.build file:

> # XXX: We should fix these warnings.
> ALLOW_COMPILER_WARNINGS = True

Comment 3

[Nicholas Nethercote [:njn]]

(In reply to Nicholas Nethercote [:njn] from comment #2)
> I was wondering how this happened, and I see that widget/cocoa/ is one of
> only two directories in the entire tree (the other being
> config/external/nss) where we still have this in the moz.build file:
> 
> > # XXX: We should fix these warnings.
> > ALLOW_COMPILER_WARNINGS = True

I just tried removing these lines in a local build. Once I fixed the warning that is the subject of this bug, the build succeeded. cpeterson, can you try doing that on try server? If it works, you can remove them in your patch and then we'll have one more guaranteed warning-free directory :)

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Stephen A Pohl [:spohl] from comment #1)
> nit: this line should be broken up to fit within the 80 character per line
> limit. It's currently at 82.

Thanks, Stephen! I'll fix that line before landing.

(In reply to Nicholas Nethercote [:njn] from comment #3)
> I just tried removing these lines in a local build. Once I fixed the warning
> that is the subject of this bug, the build succeeded. cpeterson, can you try
> doing that on try server? If it works, you can remove them in your patch and
> then we'll have one more guaranteed warning-free directory :)

I conveniently have a Try build from a few weeks ago that shows widget/cocoa is still not warning-free:

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=6680262edd88b62e37635db015146188c5ac23c3

There are some ObjC warnings-as-errors:

  widget/cocoa/nsChildView.mm:3605:10: error: 'NSView' may not respond to 'viewDidChangeBackingProperties' [-Werror]

Like you, I don't see these warnings locally. The Try builders use OS X 10.7, so perhaps they use an older Xcode SDK that doesn't know about the viewDidChangeBackingProperties method. The NSView docs says this method was added in OS X 10.7:

  https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSView_Class/index.html#//apple_ref/occ/instm/NSView/viewDidChangeBackingProperties

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/00491e46d24b
Fix -Wunused-result warning in widget/cocoa/nsCocoaWindow.mm. r=spohl

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/00491e46d24b