INSTALLED_HEADERS and you
Jeff Walden
Some headers clearly must be in that list -- jsapi.h, jsdbgapi.h, jspubtd.h, for example. Some headers have no reason to be in that list, and I just removed jsnum.h (again) and a dozen or so other such headers from the list, with a slightly-greater-than-minor amount of effort. Then there's a bulk which shouldn't be there, but for XPConnect and similar incest. It'll take awhile to work through those, but if we make a little effort here, a little effort there, consistently, I think we can clear it out. In the meantime:
Don't add new headers to INSTALLED_HEADERS!
There will probably be exceptions. But often there's a better approach -- say, moving a method definition to an inlines.h. And it's hard to claw these things back from Gecko (I'm looking at you, jsobj.h/jsscope.h/jscntxt.h). So if you want to add a header, you should get positive feedback from multiple peers before doing so.
Jeff
Brendan Eich
> Back a few weeks ago or so I added jsnum.h to the INSTALLED_HEADERS list. It turned out that header had been removed from that list, deliberately, so that Gecko couldn't use it. (Such dependencies slow down the build quite a bit for purely-JS-internal changes.)
>
> Some headers clearly must be in that list -- jsapi.h, jsdbgapi.h, jspubtd.h, for example. Some headers have no reason to be in that list, and I just removed jsnum.h (again) and a dozen or so other such headers from the list, with a slightly-greater-than-minor amount of effort. Then there's a bulk which shouldn't be there, but for XPConnect and similar incest. It'll take awhile to work through those, but if we make a little effort here, a little effort there, consistently, I think we can clear it out. In the meantime:
>
> Don't add new headers to INSTALLED_HEADERS!
Thanks for doing this -- it is a good fight we should fight.
> There will probably be exceptions. But often there's a better approach -- say, moving a method definition to an inlines.h. And it's hard to claw these things back from Gecko (I'm looking at you, jsobj.h/jsscope.h/jscntxt.h).
Yet all three of those have js*inlines.h counterparts. Maybe fixing these bad boys is just a little more little more effort?
> So if you want to add a header, you should get positive feedback from multiple peers before doing so.
+1
/be
Jeff Walden
>> There will probably be exceptions. But often there's a better approach -- say, moving a method definition to an inlines.h. And it's hard to claw these things back from Gecko (I'm looking at you, jsobj.h/jsscope.h/jscntxt.h).
>
> Yet all three of those have js*inlines.h counterparts. Maybe fixing these bad boys is just a little more little more effort?
Sometimes, plausibly, but generally not (although I admit to not skimming the full scope of tainting for every header I didn't remove). They want stuff like JSObject::setSlot and similar. My case was just that I added a method to jsnum.h, then included that in another header rather than put the inline in inlines.h, then use the inlines.h from the single cpp file that wanted it. That was easy to fix. Alas, wanting JSObject::setSlot, and other similar guts-ful things like that, is not.
Jeff
Luke Wagner
I've wondered if there is a tool for automatically generating diagrams
(say with graphviz) of #includes. It might be nice to have this so we
could make some attempt at stratification.
> Back a few weeks ago or so I added jsnum.h to the INSTALLED_HEADERS
> list. It turned out that header had been removed from that list,
> deliberately, so that Gecko couldn't use it. (Such dependencies slow
> down the build quite a bit for purely-JS-internal changes.)
>
> Some headers clearly must be in that list -- jsapi.h, jsdbgapi.h,
> jspubtd.h, for example. Some headers have no reason to be in that
> list, and I just removed jsnum.h (again) and a dozen or so other such
> headers from the list, with a slightly-greater-than-minor amount of
> effort. Then there's a bulk which shouldn't be there, but for
> XPConnect and similar incest. It'll take awhile to work through
> those, but if we make a little effort here, a little effort there,
> consistently, I think we can clear it out. In the meantime:
>
> Don't add new headers to INSTALLED_HEADERS!
>
> There will probably be exceptions. But often there's a better
> approach -- say, moving a method definition to an inlines.h. And it's
> hard to claw these things back from Gecko (I'm looking at you,
> jsobj.h/jsscope.h/jscntxt.h). So if you want to add a header, you
> should get positive feedback from multiple peers before doing so.
>
> Jeff
> _______________________________________________
> dev-tech-js-engine-internals mailing list
> dev-tech-js-en...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
Nicholas Nethercote
> Agreed. A few times I've tangled a bit with the #include clique and I've
> wondered if there is a tool for automatically generating diagrams (say with
> graphviz) of #includes. It might be nice to have this so we could make some
> attempt at stratification.
https://bugzilla.mozilla.org/show_bug.cgi?id=634839 might be of
interest. There's a Dutch CS student who dived into it with great
gusto. He's been quiet lately because he has exams, but I'm hoping
he'll get back into it soon.
Nick