553997 - JS_ISSPACE now uses js_A, js_X and js_Y which are not exported

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Mike Hommey [:glandium]]

Attachment: Patch

gxine uses JS_ISSPACE, and since 1.9.2, JS_ISSPACE uses JS_CCODE, which uses js_A, js_X and js_Y. These are not exported from the library.

Also note that while gxine doesn't use JS_ISWORD, this macro has the same problem with js_alnum, which is not exported either. While the patch I'm attaching doesn't export js_alnum, maybe it should be, too.

Comment 1

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

gxine shouldn't be using JS_ISSPACE, it's not part of the public API. why are they including jsstr.h at all?

Comment 2

[Mike Hommey [:glandium]]

Short answer: because all headers from js/src are installed as system headers.

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Hmm, I don't think we should export and maintain internal API because of packaging infelicities.  The gxine guys should find something else to use for this functionality.  Thanks for the report, though!

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 433900 [details] [diff] [review]
Patch

r-: don't want to add to the export table or in general support embedding use of internal headers.

Comment 5

[Mike Hommey [:glandium]]

(In reply to comment #4)
> (From update of attachment 433900 [details] [diff] [review])
> r-: don't want to add to the export table or in general support embedding use
> of internal headers.

Where is the distinction made between internal and public headers ?

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Not clearly enough, which is our fault.  The published header set is "js*api.h and everything they include", but embeddings shouldn't directly include any file that isn't js*api.h (that is, they shouldn't include jspubtd.h or such directly).

Comment 7

[Mike Hommey [:glandium]]

(In reply to comment #6)
> Not clearly enough, which is our fault.  The published header set is "js*api.h
> and everything they include", but embeddings shouldn't directly include any
> file that isn't js*api.h (that is, they shouldn't include jspubtd.h or such
> directly).

Shouldn't the install rules in js/src be modified accordingly, then ? (In which case I'll file another bug)

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

To be blunt, I didn't know anyone used the install rules!  Please do file (or, I fear, find already filed) a bug.

Comment 9

[Mike Hommey [:glandium]]

Before I file a new bug, could you tell me if you'd expect jsprvtd.h and jsopcode.h to be public ? That doesn't sound right, yet they are included from jsdbgapi.h and jsxdrapi.h.

Comment 10

[Mike Hommey [:glandium]]

Also note that the following headers contain JS_PUBLIC_API symbols, while they are not included from any *api.h file:
jsarena.h
jsbit.h
jsdhash.h
jsfile.h
jshash.h
jsprf.h

Comment 11

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

How about I answer comments 9 and 10 in your new bug? :)

Comment 12

[Mike Hommey [:glandium]]

Filed bug 554088