Summary: | MathExtras.h missing additional "using std::<cmathfunction>" statements | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nick Landry <nlandry> | ||||
Component: | Web Template Framework | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED INVALID | ||||||
Severity: | Normal | CC: | barraclough, buildbot, darin, dbates, dglazkov, gustavo, jturcotte, laszlo.gombos, s.mathur, webkit.review.bot, xan.lopez | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | Other | ||||||
Attachments: |
|
Description
Nick Landry
2011-01-26 11:44:00 PST
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. |