Closed Bug 918923 Opened 6 years ago Closed 6 years ago

Use nsString.h more

Categories

(Core :: XPCOM, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(2 files)

We should use it instead of nsStringGlue.h in places which only use the internal APIs.
Assignee: nobody → ehsan
Attachment #807869 - Flags: review?(benjamin)
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+
Attachment #807869 - Flags: review?(benjamin) → review+
(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. :(
Why? This breaks including those headers in code that uses the external API, right?
(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... :(
(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...
https://hg.mozilla.org/mozilla-central/rev/d1d94c5b0f40
https://hg.mozilla.org/mozilla-central/rev/c53e80370c9a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
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?
(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?
Probably because thunderbird is using the frozen API and is including these headers from external code.
Depends on: 920073
You need to log in before you can comment on or make changes to this bug.