1119071 - Clean up some code used for older unsupported MSVC versions in MFBT; r=froydnj

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Comment 1

[(no longer active)]

Attachment: Clean up some code used for older unsupported MSVC versions in MFBT

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8545578 [details] [diff] [review]
Clean up some code used for older unsupported MSVC versions in MFBT

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

::: mfbt/IntegerPrintfMacros.h
@@ -32,5 @@
>  
>  #if defined(MOZ_CUSTOM_INTTYPES_H)
>  #  include MOZ_CUSTOM_INTTYPES_H
> -#elif defined(_MSC_VER)
> -#  include "mozilla/MSIntTypes.h"

That's bug 1118529 territory.

::: mfbt/tests/TestMaybe.cpp
@@ +27,2 @@
>  // the form |decltype(foo)::type| from working. See here:
>  // http://stackoverflow.com/questions/14330768/c11-compiler-error-when-using-decltypevar-followed-by-internal-type-of-var

The comment should go next to the gcc version check.

Comment 3

[(no longer active)]

(In reply to Mike Hommey [:glandium] from comment #2)
> Comment on attachment 8545578 [details] [diff] [review]
> Clean up some code used for older unsupported MSVC versions in MFBT
> 
> Review of attachment 8545578 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mfbt/IntegerPrintfMacros.h
> @@ -32,5 @@
> >  
> >  #if defined(MOZ_CUSTOM_INTTYPES_H)
> >  #  include MOZ_CUSTOM_INTTYPES_H
> > -#elif defined(_MSC_VER)
> > -#  include "mozilla/MSIntTypes.h"
> 
> That's bug 1118529 territory.

Heh, OK!  Please review as if those parts did not exist in this patch.  :-)

> ::: mfbt/tests/TestMaybe.cpp
> @@ +27,2 @@
> >  // the form |decltype(foo)::type| from working. See here:
> >  // http://stackoverflow.com/questions/14330768/c11-compiler-error-when-using-decltypevar-followed-by-internal-type-of-var
> 
> The comment should go next to the gcc version check.

Sure, I'll do that when landing.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8545578 [details] [diff] [review]
Clean up some code used for older unsupported MSVC versions in MFBT

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

r=me with all the bits related to the IntegerPrintfMacros.h change removed.

::: mfbt/NullPtr.h
@@ +24,5 @@
>  #      define MOZ_HAVE_CXX11_NULLPTR
>  #    endif
>  #  endif
>  #elif defined(_MSC_VER)
> +   // The minimum supported MSVC (12, _MSC_VER 1800) supports nullptr.

This should be (13, _MSC_VER 1900) now, right?

Comment 5

[(no longer active)]

(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #4)
> ::: mfbt/NullPtr.h
> @@ +24,5 @@
> >  #      define MOZ_HAVE_CXX11_NULLPTR
> >  #    endif
> >  #  endif
> >  #elif defined(_MSC_VER)
> > +   // The minimum supported MSVC (12, _MSC_VER 1800) supports nullptr.
> 
> This should be (13, _MSC_VER 1900) now, right?

No.  MSVC 2013 is VC12, and it sets _MSC_VER to 1800.  Yes, there are three different version numbers.  No, they have no logical correlation to each other.  No, it's not confusing!  kthxbye :D

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1ce77cbf99f3