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)
538 bytes,
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
|
||
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
|
||
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
•