Closed
Bug 776871
Opened 12 years ago
Closed 12 years ago
Enable the use of nullptr for vc10 builds or later
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: jimm, Assigned: jimm)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
No description provided.
![]() |
Assignee | |
Comment 1•12 years ago
|
||
Seems like the right way to do this, not sure though...
Assignee: nobody → jmathies
Attachment #645251 -
Flags: review?(ehsan)
![]() |
Assignee | |
Updated•12 years ago
|
Attachment #645251 -
Attachment is patch: true
Comment 2•12 years ago
|
||
Why is this needed at all? Bug 626472 adds a configure-time check to configure.in, which defines HAVE_NULLPTR using AC_TRY_COMPILE, i.e., if "int* foo = nullptr;" compiles successfully. Does this check fail in VC versions that actually support nullptr? If so, why?
![]() |
Assignee | |
Comment 3•12 years ago
|
||
I'll take a look. HAVE_NULLPTR isn't indexed into mxr yet on mc.
![]() |
Assignee | |
Updated•12 years ago
|
Attachment #645251 -
Flags: review?(ehsan)
![]() |
Assignee | |
Comment 4•12 years ago
|
||
So $COMPILE_ENVIRONMENT is set to 1 on windows, and this sets SKIP_COMPILER_CHECKS=1, which causes configure to skip over the nullptr check.
![]() |
Assignee | |
Comment 5•12 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #4) > So $COMPILE_ENVIRONMENT is set to 1 on windows, and this sets > SKIP_COMPILER_CHECKS=1, which causes configure to skip over the nullptr > check. Hmm, so it's not COMPILE_ENVIRONMENT, SKIP_COMPILER_CHECKS is set to 1 farther up..
![]() |
Assignee | |
Comment 6•12 years ago
|
||
here it is - http://mxr.mozilla.org/mozilla-central/source/configure.in#739
![]() |
Assignee | |
Comment 7•12 years ago
|
||
will seek reviews on these once I get through a full build.
Attachment #645251 -
Attachment is obsolete: true
Comment 8•12 years ago
|
||
So it looks like Windows skips all compiler checks due to bug 58981, which landed in . . . 2002. Could we try not disabling compiler checks for Windows, maybe?
![]() |
Assignee | |
Updated•12 years ago
|
Attachment #645267 -
Flags: review?(ehsan)
![]() |
Assignee | |
Comment 9•12 years ago
|
||
(In reply to :Aryeh Gregor from comment #8) > So it looks like Windows skips all compiler checks due to bug 58981, which > landed in . . . 2002. Could we try not disabling compiler checks for > Windows, maybe? I'd rather we file a follow up on that.
Comment 10•12 years ago
|
||
No, we cannot do the compiler checks on Windows without completely rewritting AC_TRY_COMPILE which assumes a GCC-style compiler syntax.
Comment 11•12 years ago
|
||
msvc10 is supposed to support nullptr for native codes. http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.110%29.aspx http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.100%29.aspx http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.90%29.aspx Why nullptr is enabled only for msvc11?
![]() |
Assignee | |
Comment 12•12 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #11) > msvc10 is supposed to support nullptr for native codes. > http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.110%29.aspx > http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.100%29.aspx > http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.90%29.aspx > Why nullptr is enabled only for msvc11? I had no idea. I'll fire up a build with vc10 to test.
![]() |
Assignee | |
Comment 13•12 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #12) > (In reply to Masatoshi Kimura [:emk] from comment #11) > > msvc10 is supposed to support nullptr for native codes. > > http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.110%29.aspx > > http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.100%29.aspx > > http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.90%29.aspx > > Why nullptr is enabled only for msvc11? > > I had no idea. I'll fire up a build with vc10 to test. Appears to be building initially, if this makes it through I'll update the version to 1600.
Comment 14•12 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #13) > (In reply to Jim Mathies [:jimm] from comment #12) > > (In reply to Masatoshi Kimura [:emk] from comment #11) > > > msvc10 is supposed to support nullptr for native codes. > > > http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.110%29.aspx > > > http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.100%29.aspx > > > http://msdn.microsoft.com/ja-jp/library/4ex65770%28v=vs.90%29.aspx > > > Why nullptr is enabled only for msvc11? > > > > I had no idea. I'll fire up a build with vc10 to test. > > Appears to be building initially, if this makes it through I'll update the > version to 1600. Please make sure to make a preprocessed version of a source file (by doing make nsFoo.i in the corresponding objdir subdirectory for nsFoo.cpp) and check to see if nullptr is indeed being used in there.
Comment 15•12 years ago
|
||
Comment on attachment 645267 [details] [diff] [review] patch Clearing the review for now.
Attachment #645267 -
Flags: review?(ehsan)
![]() |
Assignee | |
Comment 16•12 years ago
|
||
Looks like this is giving expected output, but to be sure - generating a test.i for a test.cpp that contains: static void test() { char * p = nsnull; char * d = nullptr; } I get: (a whole bunch of white space and comments plus..) static void test() { char * p = nsnull; char * d = nullptr; }
Attachment #645267 -
Attachment is obsolete: true
Attachment #645369 -
Flags: review?(ehsan)
![]() |
Assignee | |
Comment 17•12 years ago
|
||
Or maybe this is the test you're looking for - test.cpp: static void test() { #ifndef HAVE_NULLPTR char * p = nsnull; #else char * d = nullptr; #endif } test.i: static void test() { char * d = nullptr; #line 10 "f:/Mozilla/firefox/mc/widget/windows/test.cpp" }
Updated•12 years ago
|
Attachment #645369 -
Flags: review?(ehsan) → review+
Updated•12 years ago
|
Summary: Enable the use of nullptr for vc11 builds → Enable the use of nullptr for vc10 builds or later
Comment 18•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8b5b175234df
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in
before you can comment on or make changes to this bug.
Description
•