|Product:||WebKit||Reporter:||Aron Rosenberg <arosenberg>|
|Component:||WebKit Qt||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||bfulgham, menard, net147, noam, robt_0, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description Aron Rosenberg 2011-06-29 12:11:22 PDT
Comment 1 Rob Tomek 2011-07-21 10:41:40 PDT
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.
Comment 2 Noam Rosenthal 2011-07-21 11:50:13 PDT
Is this patch for a review?
Comment 3 Aron Rosenberg 2011-07-21 11:59:17 PDT
yes, the patch should be for review
Comment 4 Noam Rosenthal 2011-07-21 12:07:21 PDT
Comment on attachment 99123 [details] patch to fix tr1 warning for VS2010 What about mingw? Shouldn't this be -win32-*:!win32-msvc2010:
Comment 5 Aron Rosenberg 2011-07-21 12:11:38 PDT
I don't have a mingw setup, so I can't answer this, but win32-icc behaves the same as msvc2010.
Comment 6 Rob Tomek 2011-07-21 13:18:04 PDT
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 7 Noam Rosenthal 2011-07-24 09:22:54 PDT
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 :)
Comment 8 Rob Tomek 2011-09-27 09:30:06 PDT
(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.
Comment 9 Brent Fulgham 2011-11-15 14:03:09 PST
Comment 10 Eric Seidel (no email) 2011-12-21 15:20:38 PST
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. :)
Comment 11 Aron Rosenberg 2012-01-13 11:53:26 PST
Created attachment 122476 [details] Remove TR1 define Added changelog entry and changed patch to remove the define based on the bug feedback
Comment 12 Alexis Menard (darktears) 2012-01-16 10:33:36 PST
Comment on attachment 122476 [details] Remove TR1 define Well it will most likely break older version of Visual Studio no?
Comment 13 Aron Rosenberg 2012-01-16 10:35:38 PST
(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 14 Simon Hausmann 2012-02-10 07:07:45 PST
Comment on attachment 122476 [details] Remove TR1 define Ok, I trust you on this one :)
Comment 15 WebKit Review Bot 2012-02-10 08:32:28 PST
Comment on attachment 122476 [details] Remove TR1 define Clearing flags on attachment: 122476 Committed r107404: <http://trac.webkit.org/changeset/107404>
Comment 16 WebKit Review Bot 2012-02-10 08:32:33 PST
All reviewed patches have been landed. Closing bug.