Closed Bug 795238 Opened 8 years ago Closed 8 years ago
Ensure nsresult is unsigned
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.
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.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #666006 - Flags: review?(ayg)
So, do I need to file a separate bug on backing out the previous MSVC-specific workarounds?
(In reply to firstname.lastname@example.org from comment #3) > So, do I need to file a separate bug on backing out the previous > MSVC-specific workarounds? Sure.
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.
Attachment #666006 - Flags: review?(ayg) → review+
Target Milestone: --- → mozilla19
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.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.