795238 - Ensure nsresult is unsigned

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[[e-mail]]

After the main nsresult enum landing, there were several places where MSVC was interpreting nsresult as signed even though its range exceeds INT_MAX. We should use MSVC's enum base type extension to force MSVC to treat it as unsigned. This should also allow us to back out the MSVC-specific workarounds.

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

That extension actually became part of the C++11 standard.  The right way to check for this is #if defined(MOZ_HAVE_CXX11_ENUM_TYPE), which currently will be true for _MSC_VER >= 1400 (see mfbt/Attributes.h).  FWIW, the C++ standard already guarantees that the underlying type in our case must be at least the size of uint32_t, so this is a bug in MSVC per spec (as well as per common sense!).

It looks like bug 794734 already added this workaround for MSVC 64-bit.  IMO, the line

  #if defined(__cplusplus) && defined(_MSC_VER) && defined(_M_X64)

should just be changed to

  #if defined(__cplusplus) && defined(MOZ_HAVE_CXX11_ENUM_TYPE)

and that should be fine.

Comment 2

[(no longer active)]

Attachment: Patch (v1)

Comment 3

[[e-mail]]

So, do I need to file a separate bug on backing out the previous MSVC-specific workarounds?

Comment 4

[(no longer active)]

(In reply to [e-mail] from comment #3)
> So, do I need to file a separate bug on backing out the previous
> MSVC-specific workarounds?

Sure.

Comment 5

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Comment on attachment 666006 [details] [diff] [review]
Patch (v1)

LGTM.  Sorry for the long delay -- I should have changed my Bugzilla name to note I was away for a week or so.

Comment 6

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/13d96461b2da

Comment 7

[(no longer active)]

Pushed a follow-up:

https://hg.mozilla.org/integration/mozilla-inbound/rev/52c773840b00

We need mozilla/Attributes.h #included there since that's where MOZ_HAVE_CXX11_ENUM_TYPE is defined.

Comment 8

[(Out Sep 17-23) Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/13d96461b2da
https://hg.mozilla.org/mozilla-central/rev/52c773840b00