Last Comment Bug 776871 - Enable the use of nullptr for vc10 builds or later
: Enable the use of nullptr for vc10 builds or later
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla17
Assigned To: Jim Mathies [:jimm]
:
Mentors:
Depends on:
Blocks: VC11 626472 777698
  Show dependency treegraph
 
Reported: 2012-07-24 04:22 PDT by Jim Mathies [:jimm]
Modified: 2012-07-26 06:33 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (493 bytes, patch)
2012-07-24 04:23 PDT, Jim Mathies [:jimm]
no flags Details | Diff | Review
patch (538 bytes, patch)
2012-07-24 05:40 PDT, Jim Mathies [:jimm]
no flags Details | Diff | Review
patch (538 bytes, patch)
2012-07-24 10:39 PDT, Jim Mathies [:jimm]
ehsan: review+
Details | Diff | Review

Description Jim Mathies [:jimm] 2012-07-24 04:22:28 PDT

    
Comment 1 Jim Mathies [:jimm] 2012-07-24 04:23:58 PDT
Created attachment 645251 [details] [diff] [review]
patch

Seems like the right way to do this, not sure though...
Comment 2 :Aryeh Gregor (away until August 15) 2012-07-24 04:32:44 PDT
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?
Comment 3 Jim Mathies [:jimm] 2012-07-24 05:01:23 PDT
I'll take a look. HAVE_NULLPTR isn't indexed into mxr yet on mc.
Comment 4 Jim Mathies [:jimm] 2012-07-24 05:20:28 PDT
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.
Comment 5 Jim Mathies [:jimm] 2012-07-24 05:29:35 PDT
(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..
Comment 6 Jim Mathies [:jimm] 2012-07-24 05:31:51 PDT
here it is - 

http://mxr.mozilla.org/mozilla-central/source/configure.in#739
Comment 7 Jim Mathies [:jimm] 2012-07-24 05:40:06 PDT
Created attachment 645267 [details] [diff] [review]
patch

will seek reviews on these once I get through a full build.
Comment 8 :Aryeh Gregor (away until August 15) 2012-07-24 06:49:24 PDT
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?
Comment 9 Jim Mathies [:jimm] 2012-07-24 07:12:53 PDT
(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 Benjamin Smedberg [:bsmedberg] 2012-07-24 07:16:25 PDT
No, we cannot do the compiler checks on Windows without completely rewritting AC_TRY_COMPILE which assumes a GCC-style compiler syntax.
Comment 11 Masatoshi Kimura [:emk] 2012-07-24 08:02:07 PDT
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?
Comment 12 Jim Mathies [:jimm] 2012-07-24 08:43:19 PDT
(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.
Comment 13 Jim Mathies [:jimm] 2012-07-24 09:05:58 PDT
(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 :Ehsan Akhgari (busy, don't ask for review please) 2012-07-24 09:19:35 PDT
(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 :Ehsan Akhgari (busy, don't ask for review please) 2012-07-24 09:20:00 PDT
Comment on attachment 645267 [details] [diff] [review]
patch

Clearing the review for now.
Comment 16 Jim Mathies [:jimm] 2012-07-24 10:39:07 PDT
Created attachment 645369 [details] [diff] [review]
patch

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;

}
Comment 17 Jim Mathies [:jimm] 2012-07-24 10:48:56 PDT
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"

}
Comment 18 Ed Morley [:emorley] 2012-07-26 05:14:14 PDT
https://hg.mozilla.org/mozilla-central/rev/8b5b175234df

Note You need to log in before you can comment on or make changes to this bug.