On 8/20/13 7:27 AM, Ehsan Akhgari wrote:
On 2013-08-19 9:10 PM, Gregory Szorc wrote:
On 8/19/13 5:15 PM, Nicholas Nethercote wrote:
Hi,
Analysis in https://bugzilla.mozilla.org/show_bug.cgi?id=901132 has
indicated that jsapi.h is probably responsible for more recompilation
than any other file in the Mozilla tree -- it gets included in *lots*
of files, and it gets modified very often, partly because it's so
large. (jsfriendapi.h is also bad in this respect.)
I'm trying to improve this situation in
https://bugzilla.mozilla.org/show_bug.cgi?id=905017. It turns out
that *many* |#include "jsapi.h"| statements are simply not needed.
They can be replaced by forward declarations of a handful of types,
e.g.:
struct jsid;
struct JSContext;
class JSObject;
namespace JS {
template <typename T> class Handle;
template <typename T> class MutableHandle;
class Value; // #include js/Value.h (not jsapi.h) if you need the
*definition*
}
Next time you're thinking of adding a |#include "jsapi.h"| statement,
please think about whether a forward declaration would suffice -- i.e.
if you are only using public JS types (i.e. not functions), and only
using them as pointers, references, or parameters in function
declarations. (Forward-declaring JS_PUBLIC_API types is harder; ask
me for help if you need to do that.)
[BTW, you might think "these popular forward declarations should be in
a header of their own, to avoid some repetition". That's a reasonable
idea, and there is a file jspubtd.h that serves this purpose... sort
of. It contains numerous forward declarations (many more than is
needed in 98% of cases), but it also defines some types, so its
purpose is muddied. So I'm not doing that for now, though creating a
new file "js/ForwardDecls.h" (or somesuch) is a possibility in the
future.]
As well as using forward declarations where suitable, I've started
breaking off small sections of jsapi.h into separate headers in
js/public/. Unfortunately, this work will only go so far because
xpcpublic.h, BindingUtils.h, and DOMJSClass.h all (unavoidably)
include jsapi.h, and they are headers that are large and included in
lots of places. I'd love to hear suggestions as to how they could be
broken into smaller pieces and/or included in fewer places.
Issues like this could be identified and prevented in the future if we
had static analysis identifying badness. Is bug 887641 the only thing
blocking unhiding the static analysis builders on TBPL (887642)? Has
anyone filed bugs to get #include sanity checking added to our Clang
plugin?
In order to do that we would basically need to build a bug-free
include-what-you-use, and AFAIK nobody has signed up to do that work.
Also note that such a check will only be useful if we adhere to the
principle of using forward-decls where possible throughout the tree,
otherwise there will be thousands of "known" failures. We're very far
from that right now.
I'm not talking about building a bug-free IWYU: I'm talking about making
the static analyzer be able to quickly identify suboptimal foo. e.g. we
would maintain a list of "please forward declare symbols from jsapi.h."
At compile time if a file includes jsapi.h and only uses symbols from
the "please forward declare" list, then a warning is issued. We can put
the additional checks behind compiler flags/warnings and can ratchet up
the strictness over time. How will this not work?
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
