Closed
Bug 918923
Opened 11 years ago
Closed 11 years ago
Use nsString.h more
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(2 files)
10.25 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
61.40 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
We should use it instead of nsStringGlue.h in places which only use the internal APIs.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #807868 -
Flags: review?(benjamin)
Assignee | ||
Comment 2•11 years ago
|
||
Assignee: nobody → ehsan
Attachment #807869 -
Flags: review?(benjamin)
Comment 3•11 years ago
|
||
Comment on attachment 807868 [details] [diff] [review] Part 1: Fix the include-what-you-use pragmas in the string headers The bit in nsStringGlue.h is not technically correct, but I'm not sure whether there is a way to correctly tell IWYU the thing we actually want: "When compiling code in xpcom/glue, the correct toplevel header is nsStringGlue.h, otherwise its nsString.h".
Attachment #807868 -
Flags: review?(benjamin) → review+
Updated•11 years ago
|
Attachment #807869 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #3) > Comment on attachment 807868 [details] [diff] [review] > Part 1: Fix the include-what-you-use pragmas in the string headers > > The bit in nsStringGlue.h is not technically correct, but I'm not sure > whether there is a way to correctly tell IWYU the thing we actually want: > > "When compiling code in xpcom/glue, the correct toplevel header is > nsStringGlue.h, otherwise its nsString.h". Yeah, I don't think that's possible. :(
Assignee | ||
Comment 5•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1d94c5b0f40 https://hg.mozilla.org/integration/mozilla-inbound/rev/c53e80370c9a
Comment 6•11 years ago
|
||
Why? This breaks including those headers in code that uses the external API, right?
Assignee | ||
Comment 7•11 years ago
|
||
(In reply to comment #6) > Why? This breaks including those headers in code that uses the external API, > right? Yeah, it's a delicate game figuring out which one of the APIs the code uses, especially in headers which can be included in standalone tests for example... :(
Assignee | ||
Comment 8•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1d94c5b0f40 https://hg.mozilla.org/integration/mozilla-inbound/rev/c53e80370c9a
Comment 9•11 years ago
|
||
(In reply to Ehsan Akhgari from comment #7) > (In reply to comment #6) > > Why? This breaks including those headers in code that uses the external API, > > right? > > Yeah, it's a delicate game figuring out which one of the APIs the code uses, > especially in headers which can be included in standalone tests for > example... :( I've already had to revert nsIWidget.h once recently...
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d1d94c5b0f40 https://hg.mozilla.org/mozilla-central/rev/c53e80370c9a
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment 11•11 years ago
|
||
The changes that I would like to see backed out are as follows: imgRequest.h* nsICharsetConverterManger.idl** nsIDateTimeFormat.h nsIPlatformCharset.h nsIScriptContext.h nsIWidget.h *not used directly but imgLoader.h includes it... **I don't actually see how this uses nsString.h?
Assignee | ||
Comment 12•11 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #11) > The changes that I would like to see backed out are as follows: > imgRequest.h* > nsICharsetConverterManger.idl** > nsIDateTimeFormat.h > nsIPlatformCharset.h > nsIScriptContext.h > nsIWidget.h > > *not used directly but imgLoader.h includes it... > **I don't actually see how this uses nsString.h? Why?
Comment 13•11 years ago
|
||
Probably because thunderbird is using the frozen API and is including these headers from external code.
You need to log in
before you can comment on or make changes to this bug.
Description
•