WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
158529
Improve code generator for functions with variadic parameters
https://bugs.webkit.org/show_bug.cgi?id=158529
Summary
Improve code generator for functions with variadic parameters
Nael Ouedraogo
Reported
2016-06-08 09:25:41 PDT
Improve code generator for functions with variadic parameters.
Attachments
Patch
(14.81 KB, patch)
2016-06-08 09:39 PDT
,
Nael Ouedraogo
no flags
Details
Formatted Diff
Diff
Patch for landing
(15.62 KB, patch)
2016-06-13 06:07 PDT
,
Nael Ouedraogo
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Nael Ouedraogo
Comment 1
2016-06-08 09:39:33 PDT
Created
attachment 280811
[details]
Patch
Nael Ouedraogo
Comment 2
2016-06-08 09:41:11 PDT
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.
Darin Adler
Comment 3
2016-06-09 09:35:02 PDT
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.
Nael Ouedraogo
Comment 4
2016-06-10 08:21:07 PDT
> 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.
Nael Ouedraogo
Comment 5
2016-06-13 06:07:45 PDT
Created
attachment 281170
[details]
Patch for landing
WebKit Commit Bot
Comment 6
2016-06-13 06:38:50 PDT
Comment on
attachment 281170
[details]
Patch for landing Clearing flags on attachment: 281170 Committed
r201988
: <
http://trac.webkit.org/changeset/201988
>
WebKit Commit Bot
Comment 7
2016-06-13 06:38:55 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug