1724553 - js/CharacterEncoding.h must be included prior to String.h due to missing types

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Evan Welsh [:ewlsh]]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:89.0) Gecko/20100101 Firefox/89.0

Steps to reproduce:

Include js/String.h without js/CharacterEncoding.h in GJS

(We utilize IWYU, so it warns if you include js/CharacterEncoding.h because it isn't directly used in the file)

Actual results:

The build broke...

In file included from ../gi/repo.cpp:25:
/usr/include/mozjs-91/js/String.h:55:30: error: ‘ConstUTF8CharsZ’ in namespace ‘JS’ does not name a type
55 | JSContext* cx, const JS::ConstUTF8CharsZ s);
| ^~~~~~~~~~~~~~~
/usr/include/mozjs-91/js/String.h:58:64: error: ‘UTF8Chars’ in namespace ‘JS’ does not name a type
58 | const JS::UTF8Chars s);
| ^~~~~~~~~

https://gitlab.gnome.org/GNOME/gjs/-/jobs/1441786

Expected results:

JS::ConstUTF8CharsZ and JS::UTF8Chars should be defined in js/String.h or js/String.h should include the appropriate header itself.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724553 - Fix include files in js/public/String.h. r?tcampbell!

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/893d447cf3cf
Fix include files in js/public/String.h. r=tcampbell

Comment 3

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/893d447cf3cf

Comment 4

[Philip Chimento [:ptomato]]

Comment on attachment 9235509 [details]
Bug 1724553 - Fix include files in js/public/String.h. r?tcampbell!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Convenience for SpiderMonkey embedders
• User impact if declined: Without this patch, js/String.h cannot be #include'd by itself, embedders will have to forward-declare JS::ConstUTF8CharsZ and JS::UTF8Chars every time they include it, or make sure js/CharacterEncoding.h is #include'd before it.
• Fix Landed on Version: 93
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): If this patch were to cause any problems, they would show up at compile time.
• String or UUID changes made by this patch: None

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9235509 [details]
Bug 1724553 - Fix include files in js/public/String.h. r?tcampbell!

Approved for 91.3esr.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/4763c2bc0e1c