865327 - NullPtr.h:40:13: warning: 'nullptr' macro redefined (clang++ -stdlib=libc++)

Status: RESOLVED INVALID

(Core :: MFBT, defect)

Description

[Jan Beich]

libc++ uses emulated nullptr_t when building without -std=c++11
which leads to a conflict with mfbt define for libstdc++.

  dist/include/mozilla/NullPtr.h:40:13: warning: 'nullptr' macro redefined
  #    define nullptr __null
	      ^
  /usr/include/c++/v1/cstddef:87:9: note: previous definition is here
  #define nullptr _VSTD::__get_nullptr_t()
	  ^
  1 warning generated.

There're a lot of such warnings under js/src.

Comment 1

[Jan Beich]

Attachment: prefer emulated

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 741398 [details] [diff] [review]
prefer emulated

Review of attachment 741398 [details] [diff] [review]:
-----------------------------------------------------------------

I think this is ok. However, won't it break if you include NullPtr.h before a libc++ header?

Comment 3

[Jeff Walden [:Waldo]]

We include a couple headers via the command line, before anything else.  Perhaps this is another one we should add to that set.

Comment 4

[Jan Beich]

Attachment: prefer emulated, v2

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #3)
> We include a couple headers via the command line, before anything else. 
> Perhaps this is another one we should add to that set.

Then libc++ build (in non-c++11 mode) may miss emulated decltype in
parser/html/jArray.h due to MOZ_HAVE_CXX11_NULLPTR not being defined.

Shouldn't happen with v2.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> I think this is ok. However, won't it break if you include NullPtr.h before
> a libc++ header?

Not with clang parser.

  $ cat -n test.cc
     1	#ifdef mozilla_NullPtr_h_
     2	#warning "mozilla/NullPtr.h" was included before
     3	#endif
     4	
     5	#include <cstddef>
     6	
     7	#ifndef MOZ_HAVE_CXX11_NULLPTR
     8	#error NullPtr.h used fallback
     9	#endif
    10	
    11	int main()
    12	{
    13	  int foo = 5 | nullptr;
    14	  return 0;
    15	}

  # before (0th line or -include)
  $ c++ -I dist/include -stdlib=libc++ test.cc
  test.cc:4:2: warning: "mozilla/NullPtr.h" was included before
	[-W#warnings]
  #warning "mozilla/NullPtr.h" was included before
   ^
  test.cc:10:2: error: NullPtr.h used fallback
  #error NullPtr.h used fallback
   ^
  test.cc:15:15: error: invalid operands to binary expression ('int' and
	'std::__1::nullptr_t')
    int foo = 5 | nullptr;
	      ~ ^ ~~~~~~~
  1 warning and 2 errors generated.

  # after <cstddef> (6th line)
  $ c++ -I dist/include -stdlib=libc++ test.cc
  test.cc:14:15: error: invalid operands to binary expression ('int' and
	'std::__1::nullptr_t')
    int foo = 5 | nullptr;
	      ~ ^ ~~~~~~~
  1 error generated.

  $ c++ -v
  FreeBSD clang version 3.3 (trunk 178860) 20130405
  Target: x86_64-unknown-freebsd10.0
  Thread model: posix

  # a bit debugging of "before" case
  $ c++ -E -I dist/include -stdlib=libc++ test.cc 2>/dev/null |
    sed -n '1,/cstddef/p'
  # 1 "test.cc"
  # 1 "<built-in>" 1
  # 1 "<built-in>" 3
  # 162 "<built-in>" 3
  # 1 "<command line>" 1
  # 1 "<built-in>" 2
  # 1 "test.cc" 2
  # 1 "dist/include/mozilla/NullPtr.h" 1
  # 14 "dist/include/mozilla/NullPtr.h"
  # 1 "dist/include/mozilla/Compiler.h" 1
  # 15 "dist/include/mozilla/NullPtr.h" 2
  # 2 "test.cc" 2





  # 1 "/usr/include/c++/v1/cstddef" 1 3

Comment 5

[(no longer active)]

(In reply to comment #3)
> We include a couple headers via the command line, before anything else. 
> Perhaps this is another one we should add to that set.

Yes, this is the correct fix here.  I don't believe that we can protect against all possible edge cases by just hacking NullPtr.h itself.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 741579 [details] [diff] [review]
prefer emulated, v2

I'm ok with this but I'll defer the real review to waldo.

Comment 7

[Jeff Walden [:Waldo]]

Comment on attachment 741579 [details] [diff] [review]
prefer emulated, v2

Review of attachment 741579 [details] [diff] [review]:
-----------------------------------------------------------------

I think we probably do still just want to put NullPtr.h on the command line, but I also don't actually see anything this is going to get wrong.  So let's run with this for now.

Comment 8

[Jeff Walden [:Waldo]]

It's worth noting that when we have enough compiler support to assume nullptr, we want to be able to just remove NullPtr.h entirely.  The #include <cstddef> may get in the way of that, by enabling people to bootleg its dependencies.  But this was brought up when the Compiler.h include was added, and at the time it was decided just to deal with that problem when we have to, so I think we can do the same here.

Comment 9

[Jan Beich]

Not an issue after bug 877937 and bug 895915.