Boris Zbarsky [:bzbarsky] Assignee
	Description • 11 years ago

Even near-empty binding files include lots of stuff.  We should stop doing that.

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 5 • 11 years ago

Comment on attachment 795830 [details] [diff] [review]
part 1.  Refactor callForEachType to return a generator so we can use it more easily, and rename it to getAllTypes.

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

::: dom/bindings/Codegen.py
@@ +609,5 @@
>          # Now find all the things we'll need as arguments because we
>          # need to wrap or unwrap them.
>          bindingHeaders = set()
>          declareIncludes = set(declareIncludes)
> +        def addHeadersForType(typeInfo):

Could be

  def addHeadersForType((t, descriptor, dictionary)):

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 6 • 11 years ago

Comment on attachment 795830 [details] [diff] [review]
part 1.  Refactor callForEachType to return a generator so we can use it more easily, and rename it to getAllTypes.

Not a fan of generators, but ok.

And Ms2ger's suggestion is good.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 7 • 11 years ago

Comment on attachment 795831 [details] [diff] [review]
part 2.  Refactor our computation of whether we need the nsDOMQS header to make it easier to do similar other things.


>@@ -852,16 +852,19 @@ def UnionConversions(descriptors, dictio
>             for f in t.flatMemberTypes:
>                 f = f.unroll()
>                 if f.isInterface():
>                     if f.isSpiderMonkeyInterface():
>                         headers.add("jsfriendapi.h")
>                         headers.add("mozilla/dom/TypedArray.h")
>                     elif not f.inner.isExternal():
>                         headers.add(CGHeaders.getDeclarationFilename(f.inner))
>+                    if (f.isGeckoInterface() and
>+                        providers[0].getDescriptor(f.inner.identifier.name).hasXPConnectImpls):
>+                        headers.add("nsDOMQS.h")
[0] could use some comment. It is rather magical and one needs to look at getRelevantProviders to
understand what it means.


> 
>         # Add header includes.
>+        bindingHeaders = [header for (header, include) in
>+                          bindingHeaders.iteritems() if include]
Not exactly easy to parse. This syntax nicely splits the important parts aside; header in the beginning and
'if include' at the end...
But given this is python, I don't expect anything better :)

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 8 • 11 years ago

Comment on attachment 795832 [details] [diff] [review]
part 3.  Move our various conditional headers to the new bindingHeaders setup.

nice

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 9 • 11 years ago

Comment on attachment 795833 [details] [diff] [review]
part 4.  Make previously-unconditional headers conditional.


>         curr = CGList([CGForwardDeclarations(config, descriptors,
>                                              mainCallbacks, workerCallbacks,
>                                              dictionaries,
>                                              callbackDescriptors + jsImplemented),
>-                       CGWrapper(CGGeneric("using namespace mozilla::dom;"),
>-                                 defineOnly=True),
Surprising, but if it compiles...

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 10 • 11 years ago

>  def addHeadersForType((t, descriptor, dictionary)):

Oh, spiffy.

> [0] could use some comment.

Will do.

> Surprising, but if it compiles...

It does, because everything after this point is wrapped in "namespace mozilla { namespace dom {".

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 11 • 11 years ago

   https://hg.mozilla.org/integration/mozilla-inbound/rev/c361190e59d7
   https://hg.mozilla.org/integration/mozilla-inbound/rev/a60adae598b8
   https://hg.mozilla.org/integration/mozilla-inbound/rev/7d19c042913d
   https://hg.mozilla.org/integration/mozilla-inbound/rev/052f30e2182b

	Ed Morley [:emorley]
	Comment 12 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/c361190e59d7
https://hg.mozilla.org/mozilla-central/rev/a60adae598b8
https://hg.mozilla.org/mozilla-central/rev/7d19c042913d
https://hg.mozilla.org/mozilla-central/rev/052f30e2182b