Closed
Bug 911020
Opened 11 years ago
Closed 11 years ago
Introduce js/TypeDecls.h, which holds very commonly used JS engine type declarations.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: n.nethercote, Assigned: n.nethercote)
Details
Attachments
(1 file)
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.
Assignee | ||
Comment 1•11 years ago
|
||
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.
Attachment #797662 -
Flags: review?(luke)
Comment 2•11 years ago
|
||
(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•11 years ago
|
||
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 ?
Attachment #797662 -
Flags: review?(luke) → review+
Assignee | ||
Comment 4•11 years ago
|
||
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•11 years ago
|
||
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•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9e98958b5e50
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in
before you can comment on or make changes to this bug.
Description
•