960664 - layout/forms/nsTextControlFrame.cpp:174:7: error: call to 'abs' is ambiguous (clang -stdlib=libc++)

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Jan Beich]

Attachment: use mozilla::Abs and cast right operand of '>' to unsigned int

In file included from layout/forms/Unified_cpp_layout_forms1.cpp:20:
layout/forms/nsTextControlFrame.cpp:174:7: error:
      call to 'abs' is ambiguous
  if (std::abs(charWidth - charMaxAdvance) > nsPresContext::CSSPixelsToAppUn...
      ^~~~~~~~
/usr/include/c++/v1/cmath:659:1: note: candidate function
abs(float __x) _NOEXCEPT {return fabsf(__x);}
^
/usr/include/c++/v1/cmath:663:1: note: candidate function
abs(double __x) _NOEXCEPT {return fabs(__x);}
^
/usr/include/c++/v1/cmath:667:1: note: candidate function
abs(long double __x) _NOEXCEPT {return fabsl(__x);}
^
1 error generated.

while mfbt/ version leads to a warning (without cast)

layout/forms/nsTextControlFrame.cpp:174:48: warning:
      comparison of integers of different signs: 'typename
      detail::AbsReturnType<int>::Type' (aka 'unsigned int') and 'nscoord'
      (aka 'int') [-Wsign-compare]
  ...mozilla::Abs(charWidth - charMaxAdvance) > nsPresContext::CSSPixelsToAppUnits(1)) {
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

Comment 1

[Jan Beich]

Attachment: explicitly include <cstdlib> for int abs(int)

gcc/libstdc++ implicitly includes <cstdlib> via <algorithm>

Comment 2

[Jan Beich]

only one of the patches needs r+

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8361226 [details] [diff] [review]
explicitly include <cstdlib> for int abs(int)

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

Thanks!

Comment 4

[Jan Beich]

r+ patch to land as commit "Bug 960664 - Unbreak libc++ build after 358f4a9aac18 (bug 157846) by using mozilla::Abs. r=roc"

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3687b6b2e48d

Thanks for the patch, Jan. Please make sure you future patches have the commit information included (at least the name/email since you had a comment with the commit message). Thanks :)
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3687b6b2e48d