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.
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.
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.
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.
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.
Attachment 80214 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7578364
Attachment 80214 [details] did not build on win: Build output: http://queues.webkit.org/results/7531341
Attachment 80214 [details] did not build on qt: Build output: http://queues.webkit.org/results/7627333
Attachment 80214 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7528338
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.
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! >
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>.
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.
Nick, do you still have an interest to resolve this ? Are you using RVCT ?
(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 ?
No response in over a year, marking resolved.