911020 - Introduce js/TypeDecls.h, which holds very commonly used JS engine type declarations.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Forward declarations of classes like |JSContext| and |JS::Value| are great in
Gecko.  But we've decided to use typedefs like |JS::HandleObject| everywhere,
so Gecko will need to see those typedefs.

Since lots of Gecko code only needs type declarations for a small number of
SpiderMonkey types (e.g. JSObject, Value, JSContext, HandleObject, HandleValue,
jschar), I think it's worth having a header for such type declarations.

"What about jspubtd.h?" I hear you ask.  Well, jspubtd.h provides much the same
service, but it's grown far too big, IMO.  It has lots of types that don't need
this level of exposure, and even defines several classes and structs.  Ugh.  So
I want a new, slimmer header of this type, and this can be viewed as a first
step towards eliminating jspubtd.h.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Introduce js/TypeDecls.h, which holds very commonly used JS engine type declarations.

This patch introduces js/TypeDecls.h, which contains a bespoke, hand-selected
collection of only 22 of the finest, most widely-applicable type declarations
that SpiderMonkey has to offer.

The patch also replaces most forward declarations of any of those 22 types with
a #include of TypeDecls.h.  There were also a few files where I could remove
the forward decls without including TypeDecls.h, e.g. because the type was no
longer used.

Since |jsid| is one of the 22 types, I was able to remove js/IdForward.h, which
is nice.

I also got rid of |HandleModule|, because it's not used, and moved
|RootedModule|'s definition to builtin/Module.cpp.  And I moved
|{Handle,Rooted}ScriptSource| to gc/Rooting.h, because they don't need to be
visible outside SM.

The End.

Comment 2

[Luke Wagner [:luke]]

(In reply to Nicholas Nethercote [:njn] from comment #1)
> This patch introduces js/TypeDecls.h, which contains a bespoke, hand-selected
> collection of only 22 of the finest, most widely-applicable type declarations
> that SpiderMonkey has to offer.

lol.  Seriously, though, we'll need to hold the line to keep TypeDecls.h slim.  And now our watch begins.

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 797662 [details] [diff] [review]
Introduce js/TypeDecls.h, which holds very commonly used JS engine type declarations.

Review of attachment 797662 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/public/TypeDecls.h
@@ +16,5 @@
> +
> +#ifndef js_TypeDecls_h
> +#define js_TypeDecls_h
> +
> +#include <stdint.h>

Can you follow this by
#ifndef UINT32_MAX
# error "__STDC_LIMIT_MACROS must be defined before including this header"
#endif
?

Comment 4

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9e98958b5e50


> ::: js/public/TypeDecls.h
> @@ +16,5 @@
> > +
> > +#ifndef js_TypeDecls_h
> > +#define js_TypeDecls_h
> > +
> > +#include <stdint.h>
> 
> Can you follow this by
> #ifndef UINT32_MAX
> # error "__STDC_LIMIT_MACROS must be defined before including this header"
> #endif
> ?

I tried this but I got build failures on the B2G builds.  Since this file doesn't use UINT32_MAX or related constants, I just omitted it.  We've got bug 909576 open for that general problem.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This landed a lot of changes to Gecko that weren't reviewed by a relevant peer. I don't think we actually want those changes.

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9e98958b5e50