Bug 109899 - [V8] MAYBE_MISSING_PARAMETER() macro is overkilling
Summary: [V8] MAYBE_MISSING_PARAMETER() macro is overkilling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-14 23:24 PST by Kentaro Hara
Modified: 2013-02-19 12:49 PST (History)
10 users (show)

See Also:


Attachments
Patch (55.57 KB, patch)
2013-02-14 23:26 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
patch for landing (56.26 KB, patch)
2013-02-18 23:38 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2013-02-14 23:24:28 PST
Currently:

- MAYBE_MISSING_PARAMETER(args, index, DefaultIsUndefined) returns args[index].
- MAYBE_MISSING_PARAMETER(args, index, DefaultIsNullString) returns Local<Value>() if args[index] is missing (i.e. the length of |args| is less than |index|). It returns args[index] otherwise.

No one other than CodeGeneratorV8.pm uses MAYBE_MISSING_PARAMETER(args, index, DefaultIsUndefined). Instead, we simply use args[index]. We should remove the redundant usage in CodeGeneratorV8.pm too. The long-name macro has been making generated code less readable.

In addition, we can rename MAYBE_MISSING_PARAMETER() to MissingIsNullString().
Comment 1 Kentaro Hara 2013-02-14 23:26:26 PST
Created attachment 188488 [details]
Patch
Comment 2 Adam Barth 2013-02-15 09:09:40 PST
Comment on attachment 188488 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188488&action=review

> Source/WebCore/bindings/v8/V8BindingMacros.h:55
> +#define MissingIsNullString(args, index) \

Can we make this an inline function instead of a pre-processor macro?  (If not, we should keep the ALL_CAPS name to let callers know that it's a macro.)
Comment 3 Kentaro Hara 2013-02-18 23:38:49 PST
Created attachment 189004 [details]
patch for landing
Comment 4 Kentaro Hara 2013-02-18 23:39:30 PST
(In reply to comment #2)
> Can we make this an inline function instead of a pre-processor macro?  (If not, we should keep the ALL_CAPS name to let callers know that it's a macro.)

Changed it to an inline function and renamed it to argumentOrNull(). Thanks.
Comment 5 WebKit Review Bot 2013-02-19 00:23:40 PST
Comment on attachment 189004 [details]
patch for landing

Clearing flags on attachment: 189004

Committed r143305: <http://trac.webkit.org/changeset/143305>
Comment 6 WebKit Review Bot 2013-02-19 00:23:43 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Yury Semikhatsky 2013-02-19 00:51:42 PST
(In reply to comment #5)
> (From update of attachment 189004 [details])
> Clearing flags on attachment: 189004
> 
> Committed r143305: <http://trac.webkit.org/changeset/143305>

This changed causes lots of warnings in the console during code generation from IDLs:

...
Use of uninitialized value $optional in string eq at ../bindings/scripts/CodeGeneratorV8.pm line 1815.
Use of uninitialized value $optional in string eq at ../bindings/scripts/CodeGeneratorV8.pm line 1832.
Use of uninitialized value $optional in string eq at ../bindings/scripts/CodeGeneratorV8.pm line 1832.
Use of uninitialized value $optional in string eq at ../bindings/scripts/CodeGeneratorV8.pm line 1832.
Use of uninitialized value $optional in string eq at ../bindings/scripts/CodeGeneratorV8.pm line 1832.
Use of uninitialized value $optional in string eq at ../bindings/scripts/CodeGeneratorV8.pm line 1832.
Use of uninitialized value $optional in string eq at ../bindings/scripts/CodeGeneratorV8.pm line 1832.
...
Comment 8 Kentaro Hara 2013-02-19 00:59:54 PST
(In reply to comment #7)
> (In reply to comment #5)
> > (From update of attachment 189004 [details] [details])
> > Clearing flags on attachment: 189004
> > 
> > Committed r143305: <http://trac.webkit.org/changeset/143305>
> 
> This changed causes lots of warnings in the console during code generation from IDLs:

Let me fix it in minutes.
Comment 9 Kentaro Hara 2013-02-19 01:02:40 PST
Fixed in r143308. Sorry for the trouble!
Comment 10 Simon Fraser (smfr) 2013-02-19 12:44:48 PST
Some Mac binding generation tests are still failing: http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/5637/steps/bindings-generation-tests/logs/stdio

Please fix.
Comment 11 Kentaro Hara 2013-02-19 12:45:17 PST
Sorry, one sec.
Comment 12 Kentaro Hara 2013-02-19 12:49:47 PST
Fixed in r143374. Sorry for the late fix!