Closed Bug 1724553 Opened 6 months ago Closed 6 months ago

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

Categories

(Core :: JavaScript Engine, defect, P1)

Firefox 91
defect

Tracking

()

RESOLVED FIXED
93 Branch
Tracking Status
firefox-esr91 --- fixed
firefox93 --- fixed

People

(Reporter: ewlsh, Assigned: arai)

Details

Attachments

(1 file)

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.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/893d447cf3cf
Fix include files in js/public/String.h. r=tcampbell
Severity: -- → S4
Priority: -- → P1
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch

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
Attachment #9235509 - Flags: approval-mozilla-esr91?

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

Approved for 91.3esr.

Attachment #9235509 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
You need to log in before you can comment on or make changes to this bug.