Closed
Bug 1126269
Opened 9 years ago
Closed 9 years ago
Cleanup nsError.h
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla38
People
(Reporter: emk, Assigned: emk)
References
Details
Attachments
(3 files)
4.64 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
7.06 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
969 bytes,
patch
|
smontagu
:
review+
|
Details | Diff | Splinter Review |
* TraceMalloc is gone that was the last C-consumer of nsresult and NS_* error codes. * Now we can assume C++11 enum class is always available.
Assignee | ||
Comment 1•9 years ago
|
||
These files include nsDebug.h or nscore.h which will end up with including nsError.h. But they don't actually use nsresult. I replaced them with the corresponding MFBT headers.
Attachment #8555208 -
Flags: review?(roc)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8555210 -
Flags: review?(nfroyd)
Comment 3•9 years ago
|
||
Comment on attachment 8555210 [details] [diff] [review] Part 3: Always use the C++11 enum class for nsresult Review of attachment 8555210 [details] [diff] [review]: ----------------------------------------------------------------- Getting to remove that Makefile.in is excellent! ::: xpcom/base/nsError.h @@ +7,5 @@ > #ifndef nsError_h__ > #define nsError_h__ > > +#ifndef __cplusplus > +#error nsError.h no longer supports C sources Thank you for adding this! ::: xpcom/base/nsErrorAssertsC.c @@ -8,5 @@ > - */ > -MOZ_STATIC_ASSERT(((nsresult)0) < ((nsresult)-1), > - "nsresult must be an unsigned type"); > -MOZ_STATIC_ASSERT(sizeof(nsresult) == sizeof(uint32_t), > - "nsresult must be 32 bits"); I think it's worth keeping this file, though I suppose we'd have to rename it to nsErrorAsserts.cpp?
Attachment #8555210 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #3) > ::: xpcom/base/nsErrorAssertsC.c > @@ -8,5 @@ > > - */ > > -MOZ_STATIC_ASSERT(((nsresult)0) < ((nsresult)-1), > > - "nsresult must be an unsigned type"); > > -MOZ_STATIC_ASSERT(sizeof(nsresult) == sizeof(uint32_t), > > - "nsresult must be 32 bits"); > > I think it's worth keeping this file, though I suppose we'd have to rename > it to nsErrorAsserts.cpp? Corresponding C++ static_assertions are already included in nsError.h.
Comment 5•9 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #4) > (In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #3) > > ::: xpcom/base/nsErrorAssertsC.c > > @@ -8,5 @@ > > > - */ > > > -MOZ_STATIC_ASSERT(((nsresult)0) < ((nsresult)-1), > > > - "nsresult must be an unsigned type"); > > > -MOZ_STATIC_ASSERT(sizeof(nsresult) == sizeof(uint32_t), > > > - "nsresult must be 32 bits"); > > > > I think it's worth keeping this file, though I suppose we'd have to rename > > it to nsErrorAsserts.cpp? > > Corresponding C++ static_assertions are already included in nsError.h. Ah, very good! Ignore that bit of the review, then. :)
Attachment #8555208 -
Flags: review?(roc) → review+
Comment 6•9 years ago
|
||
Nice patches!
Assignee | ||
Comment 7•9 years ago
|
||
Thanks for the quick reviews! https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b28a6801ff8
Assignee | ||
Comment 8•9 years ago
|
||
This file is built only for non-desktop builds. It included nscore.h, but it didn't use any functionality form the header.
Attachment #8556004 -
Flags: review?(smontagu)
Assignee | ||
Updated•9 years ago
|
Attachment #8555210 -
Attachment description: Part 2: Always use the C++11 enum class for nsresult → Part 3: Always use the C++11 enum class for nsresult
Assignee | ||
Comment 9•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d64e4339027
Updated•9 years ago
|
Attachment #8556004 -
Flags: review?(smontagu) → review+
Assignee | ||
Comment 10•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/34febed8c5ea https://hg.mozilla.org/integration/mozilla-inbound/rev/b9d974cad5ca https://hg.mozilla.org/integration/mozilla-inbound/rev/b2a79c8fcfec
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Flags: in-testsuite-
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/34febed8c5ea https://hg.mozilla.org/mozilla-central/rev/b9d974cad5ca https://hg.mozilla.org/mozilla-central/rev/b2a79c8fcfec
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•