widget/gtk/nsWindow.cpp CLAMP() breaks build with gcc 4.9

RESOLVED FIXED in Firefox 61

Status

()

--
blocker
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: miketwebster, Assigned: miketwebster)

Tracking

unspecified
mozilla61
Points:
---

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

a year ago
Originally reported: https://bugzilla.mozilla.org/show_bug.cgi?id=1418749#c49

In nsWindow::SetProgress:

widget/gtk/nsWindow.cpp:7097:80: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]

This is being caused by the expansion of the CLAMP macro, which ends up checking if (progressPercent < 0)

See: https://developer.gnome.org/glib/stable/glib-Standard-Macros.html#CLAMP:CAPS

Currently blocking building of Thunderbird Dailies for linux.
(Assignee)

Updated

a year ago
Blocks: 1418749
(Assignee)

Comment 1

a year ago
Patch attached.
Attachment #8958657 - Flags: review?(karlt)
Attachment #8958657 - Flags: feedback?(jorgk)
Comment on attachment 8958657 [details] [diff] [review]
0001-Bug-1445503-widget-gtk-nsWindow.cpp-Use-MIN-instead-.patch

Thanks again!

>Subject: [PATCH] Bug 1445503 - widget/gtk/nsWindow.cpp: Use MIN instead of
> CLAMP. CLAMP is unnecessary, as the minimum acceptable value is 0, and
> progressPercent is unsigned.  This can trigger the following warning/ error
> in some builds:

Gecko convention is to have just a summary in the first line, then a blank line, then the rest of the explanation.  Something like:

Bug 1445503 - Use MIN instead of unnecessary CLAMP r=karlt

CLAMP is unnecessary as the minimum acceptable value is 0, and
progressPercent is unsigned.  CLAMP can trigger ...
Attachment #8958657 - Flags: review?(karlt)
Attachment #8958657 - Flags: review+
Attachment #8958657 - Flags: feedback?(jorgk)
Attachment #8958657 - Flags: feedback+

Comment 3

a year ago
I was going to give this a try build, but someone stole my f? ;-) So I'll fix the commit message and ask for checkin :-)
Assignee: nobody → miketwebster
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Comment 4

a year ago
Fixed commit message and added newline to the end of the file.

Carrying over Karl's r+.
Attachment #8958657 - Attachment is obsolete: true
Attachment #8958732 - Flags: review+

Updated

a year ago
Keywords: checkin-needed

Comment 5

a year ago
Pushed by dluca@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d25f89538c90
Use MIN instead of unnecessary CLAMP r=karlt
Keywords: checkin-needed

Comment 6

a year ago
Thank you very much Michael, Karl, and Jörg.
(In reply to Jorg K (GMT+1) from comment #3)
> but someone stole my f? ;-)

Sorry about that.  I guess I must be in the habit of ticking boxes.
Thanks for finishing.

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d25f89538c90
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.