Replace NS_ABS with std::abs

RESOLVED FIXED in mozilla20

Status

()

enhancement
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: mats, Assigned: mats)

Tracking

Trunk
mozilla20
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Assignee

Description

7 years ago
No description provided.
Assignee

Comment 1

7 years ago
Posted patch fixSplinter Review
std::abs(int/long) lives in <cstdlib> and std::abs(float/double) lives in
<cmath>.  At first I just included the header that I thought was needed
but I discovered that it was easy to make a mistake and use abs(double)
for an integer value, so it seems safer to always include both.
Also, because of NS_COORD_IS_FLOAT we need to have both for nscoord.

https://tbpl.mozilla.org/?tree=Try&rev=eece68407f10
Attachment #687746 - Flags: review?(roc)

Comment 2

7 years ago
comm-central uses NS_ABS in two places too: <http://mxr.mozilla.org/comm-central/search?string=NS_ABS>.  If you could remove those as well, we could completely get rid of NS_ABS.
Assignee

Comment 3

7 years ago
Comment on attachment 687746 [details] [diff] [review]
fix

Sorry, forgot comm-central... yes, my intention is to remove NS_ABS completely.
Attachment #687746 - Attachment is obsolete: true
Attachment #687746 - Flags: review?(roc)
Assignee

Updated

7 years ago
Attachment #687746 - Attachment is obsolete: false
Attachment #687746 - Flags: review?(roc)
Assignee

Comment 4

7 years ago
(not tested yet)
Attachment #687788 - Flags: review?(roc)
Assignee

Comment 5

7 years ago
Is it possible to push both patches together to Try to make a Thunderbird test run
for example?

Comment 6

7 years ago
(In reply to Mats Palmgren [:mats] from comment #5)
> Is it possible to push both patches together to Try to make a Thunderbird
> test run
> for example?

CCing some folks who would know...
Assignee

Comment 8

7 years ago
It seems it didn't pick up the changes in the mozilla/ sub-directory:
https://tbpl.mozilla.org/?usebuildbot=1&tree=Thunderbird-Try&rev=0a64e1309c05

I have one changeset in the comm-central working tree, and one changeset
in the mozilla/ sub-directory in that tree (this is a separate working tree of
m-c as I understand it).  How do I push those two changes *together* to Try?

(I used "hg push -f -rtip  ssh://hg.mozilla.org/try-comm-central" for the
push above, but that didn't work)
Assignee

Comment 9

7 years ago
Oh, I have edit some python etc.  Sorry, I didn't realize that *is* the
instructions on how to push a combined c-c/m-c patch...
Assignee

Comment 11

7 years ago
It seems try-comm-central failed to build it for some reason I don't understand.
The "[mq]: mc-817574" cset adds "--apply-patches" in build/client.py-args and
there's a new file named mozilla-817574.patch, what did I miss?

https://tbpl.mozilla.org/?usebuildbot=1&tree=Thunderbird-Try&rev=8a34e4ed19b2
(I can't find any log file, but the self-serve page says "Failure")
There's a known issue with the Mac debug builds failing to run bloat tests, so they are hidden at the moment. If you add &noignore=1 to the url you'll see the hidden builder.

The build passed, the bloat tests didn't, which is expected.
https://hg.mozilla.org/mozilla-central/rev/3712c48edfd0
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20

Updated

7 years ago
Depends on: 818391
Depends on: 818685
Depends on: 824317
You need to log in before you can comment on or make changes to this bug.