Implement variadic arguments for WebIDL methods

RESOLVED FIXED in mozilla20

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

unspecified
mozilla20
x86
Mac OS X
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

This is needed for things like the recent changes to ClassList.add/remove.
Blocks: 814014
Created attachment 685723 [details] [diff] [review]
Implement support for variadic arguments in WebIDL.
Attachment #685723 - Flags: review?(peterv)
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Comment on attachment 685723 [details] [diff] [review]
Implement support for variadic arguments in WebIDL.

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

r=me on the WebIDL.py bits.
Attachment #685723 - Flags: review+
Comment on attachment 685723 [details] [diff] [review]
Implement support for variadic arguments in WebIDL.

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

::: dom/bindings/Codegen.py
@@ +3068,5 @@
> +
> +        # NOTE: Keep this in sync with sequence conversions as needed
> +        variadicConversion = string.Template("""const ${seqType} ${declName};
> +if (${argc} > ${index}) {
> +  ${seqType} &arr = const_cast< ${seqType}& >(${declName});

I think we usually do |${seqType}& arr|.

@@ +3074,5 @@
> +  if (!arr.SetCapacity(${argc} - ${index})) {
> +    JS_ReportOutOfMemory(cx);
> +    return false;
> +  }
> +  for (uint32_t variadicArg = 0; variadicArg < length; ++variadicArg) {

How do you feel about making this go from ${index} to ${argc} (and val would just be ${argv}[variadicArg]). I think that would be slightly clearer.

@@ +6819,5 @@
>  
>  class CGNativeMember(ClassMethod):
>      def __init__(self, descriptor, member, name, signature, extendedAttrs,
>                   breakAfter=True, passCxAsNeeded=True, visibility="public",
> +                 jsObjectsArePtr=False, variadicIsSequence=False):

Could you explain variadicIsSequence, when would one pass in (the default) False?
> I think we usually do |${seqType}& arr|.

Done.

> How do you feel about making this go from ${index} to ${argc}

Done.

> Could you explain variadicIsSequence, when would one pass in (the default) False?

And have it actually matter?  Right now, for callback function codegen, which takes an nsTArray for variadics, not a Sequence.
Created attachment 689550 [details] [diff] [review]
Interdiff for the review comments
Attachment #685723 - Flags: review?(peterv) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/7a5e83065d9f
Flags: in-testsuite+
Keywords: dev-doc-needed
Whiteboard: [need review]
Target Milestone: --- → mozilla20
Documented at https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#typemapping
Keywords: dev-doc-needed → dev-doc-complete
https://hg.mozilla.org/mozilla-central/rev/7a5e83065d9f
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.