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

RESOLVED FIXED in Firefox 51

Status

()

P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

({regression})

Trunk
mozilla51
regression
Points:
---

Firefox Tracking Flags

(firefox50 unaffected, firefox51 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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);
Attachment #8783345 - Flags: review?(spohl.mozilla.bugs)
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. :-)
Attachment #8783345 - Flags: review?(spohl.mozilla.bugs) → review+
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
(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 :)
Flags: needinfo?(cpeterson)
(Assignee)

Comment 4

2 years ago
(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
Flags: needinfo?(cpeterson)

Comment 5

2 years ago
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/00491e46d24b
Fix -Wunused-result warning in widget/cocoa/nsCocoaWindow.mm. r=spohl

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/00491e46d24b
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.