| Summary: | [Qt] Visual Studio 2010 compiler warning about TR1 support in JavascriptCore | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Aron Rosenberg <arosenberg> | ||||||
| Component: | WebKit Qt | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bfulgham, menard, net147, noam, robt_0, webkit.review.bot | ||||||
| Priority: | P3 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows 7 | ||||||||
| Attachments: |
|
||||||||
Same thing here. It's a really easy fix to make (7 characters), and it would be nice to get rid of all of the compiler warnings. Is this patch for a review? yes, the patch should be for review Comment on attachment 99123 [details]
patch to fix tr1 warning for VS2010
What about mingw?
Shouldn't this be -win32-*:!win32-msvc2010:
I don't have a mingw setup, so I can't answer this, but win32-icc behaves the same as msvc2010. As far as I can tell, we can just delete that line and it should have no effect on any compiler. The only file that I can find using TR1 is wtf/TypeTraits.h, and lines 27 & 182 already have: #if (defined(__GLIBCXX__) && (__GLIBCXX__ >= 20070724) && defined(__GXX_EXPERIMENTAL_CXX0X__)) || (defined(_MSC_VER) && (_MSC_VER >= 1600)) Aron's ICC compiler shouldn't even be using the TR1 commands anyway. Also, if mingw doesn't have a gcc version with the cxx0x defined, then it doesn't use that code either. Comment on attachment 99123 [details]
patch to fix tr1 warning for VS2010
OK, those multiple Windows configs are hard to test. But if it breaks a bot, prepare for a rollout :)
(In reply to comment #7) > (From update of attachment 99123 [details]) > OK, those multiple Windows configs are hard to test. But if it breaks a bot, prepare for a rollout :) The only two compilers that _HAS_TR1 even affects are VS2008 with the feature pack installed and VS2010. It looks like at one time that they wanted to block VS2008 with the feature pack and someone added _HAS_TR1 = 0, but someone else accomplished the same thing by putting in _MSC_VER >= 1600 around the TR1 commands. The VS2008 implementation of TR1 is not fully conforming, so I'm assuming that's why it is avoided. Basically, we have two fixes for the problem in place, with one causing an enormous amount of obnoxious warnings (and downgrading the VS2010 compiler). gcc, icc, etc. will not be affected since _HAS_TR1 doesn't even do anything. Someone should just delete that line, or at least implement this patch, already. Note that since VS2010 now support nullptr_t, the JavaScriptCore.def declaration for ?nullptr@@3Vnullptr_t@std@@A must be removed. Comment on attachment 99123 [details]
patch to fix tr1 warning for VS2010
All changes require a Changelog. Sorry that's going to be so many more lines than thsi change. :)
Created attachment 122476 [details]
Remove TR1 define
Added changelog entry and changed patch to remove the define based on the bug feedback
Comment on attachment 122476 [details]
Remove TR1 define
Well it will most likely break older version of Visual Studio no?
(In reply to comment #12) > (From update of attachment 122476 [details]) > Well it will most likely break older version of Visual Studio no? See comment #8, the effected code has already been properly ifdef with _MSC_VER checks. The define in the qt file is not needed (and not done in the non-Qt builds anyway). Comment on attachment 122476 [details]
Remove TR1 define
Ok, I trust you on this one :)
Comment on attachment 122476 [details] Remove TR1 define Clearing flags on attachment: 122476 Committed r107404: <http://trac.webkit.org/changeset/107404> All reviewed patches have been landed. Closing bug. |
Created attachment 99123 [details] patch to fix tr1 warning for VS2010 When building qtwebkit-2.2 branch with Visual Studio 2010, there is a warning about _HAS_TR1 being redefined. VS 2010 fully supports TR1 so the override of the define in the JavascriptCore pri file is not needed. Attached is a patch which fixes this.