Summary: | Improve code generator for functions with variadic parameters | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nael Ouedraogo <nael.ouedp> | ||||||
Component: | Bindings | Assignee: | Nael Ouedraogo <nael.ouedp> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, cgarcia, commit-queue, darin, youennf | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 158942 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Nael Ouedraogo
2016-06-08 09:25:41 PDT
Created attachment 280811 [details]
Patch
JS bindings code generated with variadic parameters can be improved as proposed in the uploaded patch. A further possible improvement is to avoid using a Vector of pointers since each item is checked to be not null before being append to the Vector. I would like to use a Vector of "references" instead. One solution would be to use a Vector of std::reference_wrapper<T>. Is there another solution ? It seems that variadic parameters are not correctly supported in ObjC and GObject code generators. A test is added to skip functions with variadic. Comment on attachment 280811 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280811&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3666 > + push(@$outputArray, " $name.reserveInitialCapacity(state->argumentCount() - $argumentIndex);\n"); Do we have an ironclad guarantee that argumentCount() is >= $argumentIndex? It‘s critical that we know that’s guaranteed, because if not this will try to reserve a colossal capacity. > Do we have an ironclad guarantee that argumentCount() is >= $argumentIndex?
> It‘s critical that we know that’s guaranteed, because if not this will try
> to reserve a colossal capacity.
Thanks for the review.
Actually, we have no guarantee. It may happen when a variadic parameter is preceded by an optional parameter. This issue is also observed in toNativeArguments().
I will modify JS code generator so that it dies when an optional parameter is preceding a variadic. I will also add an assert before call to reserveInitialCapacity().
I am going to land a new patch with these modifications.
Created attachment 281170 [details]
Patch for landing
Comment on attachment 281170 [details] Patch for landing Clearing flags on attachment: 281170 Committed r201988: <http://trac.webkit.org/changeset/201988> All reviewed patches have been landed. Closing bug. |