Bug 63642 – [Qt] Visual Studio 2010 compiler warning about TR1 support in JavascriptCore

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Aron Rosenberg]

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.

[Rob Tomek]

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.

[Noam Rosenthal]

Is this patch for a review?

[Aron Rosenberg]

yes, the patch should be for review

[Noam Rosenthal]

Comment on attachment 99123 [details]
patch to fix tr1 warning for VS2010

What about mingw? 
Shouldn't this be -win32-*:!win32-msvc2010:

[Aron Rosenberg]

I don't have a mingw setup, so I can't answer this, but win32-icc behaves the same as msvc2010.

[Rob Tomek]

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.

[Noam Rosenthal]

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 :)

[Rob Tomek]

(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.

[Brent Fulgham]

Note that since VS2010 now support nullptr_t, the JavaScriptCore.def declaration for ?nullptr@@3Vnullptr_t@std@@A must be removed.

[Eric Seidel (no email)]

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. :)

[Aron Rosenberg]

Created attachment 122476 [details]
Remove TR1 define

Added changelog entry and changed patch to remove the define based on the bug feedback

[Alexis Menard (darktears)]

Comment on attachment 122476 [details]
Remove TR1 define

Well it will most likely break older version of Visual Studio no?

[Aron Rosenberg]

(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).

[Simon Hausmann]

Comment on attachment 122476 [details]
Remove TR1 define

Ok, I trust you on this one :)

[WebKit Review Bot]

Comment on attachment 122476 [details]
Remove TR1 define

Clearing flags on attachment: 122476

Committed r107404: <http://trac.webkit.org/changeset/107404>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.