Bug 63642 - [Qt] Visual Studio 2010 compiler warning about TR1 support in JavascriptCore
Summary: [Qt] Visual Studio 2010 compiler warning about TR1 support in JavascriptCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P3 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-29 12:11 PDT by Aron Rosenberg
Modified: 2012-02-10 08:32 PST (History)
6 users (show)

See Also:


Attachments
patch to fix tr1 warning for VS2010 (434 bytes, patch)
2011-06-29 12:11 PDT, Aron Rosenberg
eric: review-
Details | Formatted Diff | Diff
Remove TR1 define (999 bytes, patch)
2012-01-13 11:53 PST, Aron Rosenberg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aron Rosenberg 2011-06-29 12:11:22 PDT
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.
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
Note that since VS2010 now support nullptr_t, the JavaScriptCore.def declaration for ?nullptr@@3Vnullptr_t@std@@A must be removed.
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.