Bug 53183 – MathExtras.h missing additional "using std::<cmathfunction>" statements

• Component: Web Template Framework
• Status: RESOLVED INVALID

[Nick Landry]

Looking at the C++ spec, if cmath is included the only guarantee we have is that the math functions will appear in the std namespace (spec section 17.4.1.2 clause 4, http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1905.pdf).  By only including cmath in MathExtras.h, we don't have any guarantees that the math functions will also appear in the global namespace.  MSVC headers include math functions in both std and in global, but RVCT 4 does not (without special compile parameters) and will fail to compile with missing identifiers.

[Nick Landry]

Created attachment 80214 [details]
Patch adding additional "using std::<cmathfunction>"

Since MathExtras.h was changed from math.h to use cmath, I've attached a patch which simply imports additional cmath functions into the global namespace.  I'm not sure if this is the best approach.

[WebKit Review Bot]

Attachment 80214 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/wtf/MathExtras.h:215:  Use 'using namespace std;' instead of 'using std::acos;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:216:  Use 'using namespace std;' instead of 'using std::asin;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:217:  Use 'using namespace std;' instead of 'using std::atan2;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:218:  Use 'using namespace std;' instead of 'using std::atan2f;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:219:  Use 'using namespace std;' instead of 'using std::ceil;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:220:  Use 'using namespace std;' instead of 'using std::ceilf;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:221:  Use 'using namespace std;' instead of 'using std::cos;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:222:  Use 'using namespace std;' instead of 'using std::cosf;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:223:  Use 'using namespace std;' instead of 'using std::fabs;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:224:  Use 'using namespace std;' instead of 'using std::fabsf;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:225:  Use 'using namespace std;' instead of 'using std::floorf;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:226:  Use 'using namespace std;' instead of 'using std::fmod;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:227:  Use 'using namespace std;' instead of 'using std::hypot;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:228:  Use 'using namespace std;' instead of 'using std::sin;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:229:  Use 'using namespace std;' instead of 'using std::sinf;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:230:  Use 'using namespace std;' instead of 'using std::sqrt;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:231:  Use 'using namespace std;' instead of 'using std::sqrtf;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:232:  Use 'using namespace std;' instead of 'using std::tan;'.  [build/using_std] [4]
Source/JavaScriptCore/wtf/MathExtras.h:233:  Use 'using namespace std;' instead of 'using std::tanf;'.  [build/using_std] [4]
Total errors found: 19 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nick Landry]

I decided again having "using namespace std;" in a header file since I figured it could have unintended consequences for someone who includes it but may not want all of std imported.

(In reply to comment #2)
> Attachment 80214 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
> 
> Source/JavaScriptCore/wtf/MathExtras.h:215:  Use 'using namespace std;' instead of 'using std::acos;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:216:  Use 'using namespace std;' instead of 'using std::asin;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:217:  Use 'using namespace std;' instead of 'using std::atan2;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:218:  Use 'using namespace std;' instead of 'using std::atan2f;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:219:  Use 'using namespace std;' instead of 'using std::ceil;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:220:  Use 'using namespace std;' instead of 'using std::ceilf;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:221:  Use 'using namespace std;' instead of 'using std::cos;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:222:  Use 'using namespace std;' instead of 'using std::cosf;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:223:  Use 'using namespace std;' instead of 'using std::fabs;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:224:  Use 'using namespace std;' instead of 'using std::fabsf;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:225:  Use 'using namespace std;' instead of 'using std::floorf;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:226:  Use 'using namespace std;' instead of 'using std::fmod;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:227:  Use 'using namespace std;' instead of 'using std::hypot;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:228:  Use 'using namespace std;' instead of 'using std::sin;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:229:  Use 'using namespace std;' instead of 'using std::sinf;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:230:  Use 'using namespace std;' instead of 'using std::sqrt;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:231:  Use 'using namespace std;' instead of 'using std::sqrtf;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:232:  Use 'using namespace std;' instead of 'using std::tan;'.  [build/using_std] [4]
> Source/JavaScriptCore/wtf/MathExtras.h:233:  Use 'using namespace std;' instead of 'using std::tanf;'.  [build/using_std] [4]
> Total errors found: 19 in 2 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

[Nick Landry]

Should be "I decided against"

(In reply to comment #3)
> I decided again having "using namespace std;" in a header file since I figured it could have unintended consequences for someone who includes it but may not want all of std imported.
> 
> (In reply to comment #2)
> > Attachment 80214 [details] [details] did not pass style-queue:
> > 
> > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
> > 
> > Source/JavaScriptCore/wtf/MathExtras.h:215:  Use 'using namespace std;' instead of 'using std::acos;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:216:  Use 'using namespace std;' instead of 'using std::asin;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:217:  Use 'using namespace std;' instead of 'using std::atan2;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:218:  Use 'using namespace std;' instead of 'using std::atan2f;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:219:  Use 'using namespace std;' instead of 'using std::ceil;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:220:  Use 'using namespace std;' instead of 'using std::ceilf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:221:  Use 'using namespace std;' instead of 'using std::cos;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:222:  Use 'using namespace std;' instead of 'using std::cosf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:223:  Use 'using namespace std;' instead of 'using std::fabs;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:224:  Use 'using namespace std;' instead of 'using std::fabsf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:225:  Use 'using namespace std;' instead of 'using std::floorf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:226:  Use 'using namespace std;' instead of 'using std::fmod;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:227:  Use 'using namespace std;' instead of 'using std::hypot;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:228:  Use 'using namespace std;' instead of 'using std::sin;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:229:  Use 'using namespace std;' instead of 'using std::sinf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:230:  Use 'using namespace std;' instead of 'using std::sqrt;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:231:  Use 'using namespace std;' instead of 'using std::sqrtf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:232:  Use 'using namespace std;' instead of 'using std::tan;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:233:  Use 'using namespace std;' instead of 'using std::tanf;'.  [build/using_std] [4]
> > Total errors found: 19 in 2 files
> > 
> > 
> > If any of these errors are false positives, please file a bug against check-webkit-style.

(In reply to comment #3)
> I decided again having "using namespace std;" in a header file since I figured it could have unintended consequences for someone who includes it but may not want all of std imported.
> 
> (In reply to comment #2)
> > Attachment 80214 [details] [details] did not pass style-queue:
> > 
> > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
> > 
> > Source/JavaScriptCore/wtf/MathExtras.h:215:  Use 'using namespace std;' instead of 'using std::acos;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:216:  Use 'using namespace std;' instead of 'using std::asin;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:217:  Use 'using namespace std;' instead of 'using std::atan2;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:218:  Use 'using namespace std;' instead of 'using std::atan2f;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:219:  Use 'using namespace std;' instead of 'using std::ceil;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:220:  Use 'using namespace std;' instead of 'using std::ceilf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:221:  Use 'using namespace std;' instead of 'using std::cos;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:222:  Use 'using namespace std;' instead of 'using std::cosf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:223:  Use 'using namespace std;' instead of 'using std::fabs;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:224:  Use 'using namespace std;' instead of 'using std::fabsf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:225:  Use 'using namespace std;' instead of 'using std::floorf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:226:  Use 'using namespace std;' instead of 'using std::fmod;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:227:  Use 'using namespace std;' instead of 'using std::hypot;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:228:  Use 'using namespace std;' instead of 'using std::sin;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:229:  Use 'using namespace std;' instead of 'using std::sinf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:230:  Use 'using namespace std;' instead of 'using std::sqrt;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:231:  Use 'using namespace std;' instead of 'using std::sqrtf;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:232:  Use 'using namespace std;' instead of 'using std::tan;'.  [build/using_std] [4]
> > Source/JavaScriptCore/wtf/MathExtras.h:233:  Use 'using namespace std;' instead of 'using std::tanf;'.  [build/using_std] [4]
> > Total errors found: 19 in 2 files
> > 
> > 
> > If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 80214 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7578364

[Build Bot]

Attachment 80214 [details] did not build on win:
Build output: http://queues.webkit.org/results/7531341

[Early Warning System Bot]

Attachment 80214 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7627333

[WebKit Review Bot]

Attachment 80214 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7528338

[Darin Adler]

Comment on attachment 80214 [details]
Patch adding additional "using std::<cmathfunction>"

View in context: https://bugs.webkit.org/attachment.cgi?id=80214&action=review

review- because this is breaking the build on every platform!

>> Source/JavaScriptCore/wtf/MathExtras.h:215
>> +using std::acos;
> 
> Use 'using namespace std;' instead of 'using std::acos;'.  [build/using_std] [4]

The stylebot should enforce that rule in .cpp files, but not in .h files.

[Nick Landry]

Looks like most platforms don't put the C99 math functions into the std namespace like RVCT does.  I can post another patch that moves all the C99 math function using statements into a RVCT specific if/def block.  It isn't pretty though.  Are there any other alternatives?

Could we go back to just including <math.h> instead of <cmath>?



(In reply to comment #9)
> (From update of attachment 80214 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80214&action=review
> 
> review- because this is breaking the build on every platform!
>

[Daniel Bates]

CC'ing Laszlo Gombos, Siddharth Mathur, Jocelyn Turcotte for any insight/suggestions with regards to this issue <https://bugs.webkit.org/show_bug.cgi?id=53183#c10>.

[Laszlo Gombos]

Symbian - building with RVCT - does not have this build problem as the Symbian tool chain preincludes a file called "rvct.h" which makes math functions appear in the global namespace (the file is available e.g. in the SDK - http://labs.qt.nokia.com/2011/01/20/qt-sdk-1-1-technology-preview-released/).

In case we decide to import functions into the global namespace with "using std::XX" it needs to be guarded with at least !COMPILER(WINSCW) && !(COMPILER(RVCT) && OS(SYMBIAN)). 

I do not have a strong preference on including cmath vs. math.h.

[Laszlo Gombos]

Nick, do you still have an interest to resolve this ? Are you using RVCT ?

[Laszlo Gombos]

(In reply to comment #13)
> Nick, do you still have an interest to resolve this ? Are you using RVCT ?

Perhaps this was resolved by http://trac.webkit.org/changeset/101041. Can this bug be closed ?

[Alexey Proskuryakov]

No response in over a year, marking resolved.