Closed Bug 1119071 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(1 file)

No description provided.
Depends on: 1117820
Assignee: nobody → ehsan
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.
(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 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?
Attachment #8545578 - Flags: review?(nfroyd) → review+
(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
https://hg.mozilla.org/mozilla-central/rev/1ce77cbf99f3
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.