Summary: | Rename [Optional=CallWithDefalutValue] and [Optional=CallWithNullValue] | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kentaro Hara <haraken> | ||||||||
Component: | WebCore JavaScript | Assignee: | Kentaro Hara <haraken> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, darin, japhet, macpherson, menard, ojan, ossy, tkent, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 77393 | ||||||||||
Attachments: |
|
Description
Kentaro Hara
2012-02-08 21:36:39 PST
That's a good plan. Created attachment 126243 [details]
Patch
Attachment 126243 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:189: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:190: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:191: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 3 in 141 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r107182: <http://trac.webkit.org/changeset/107182> Comment on attachment 126243 [details]
Patch
Committed manually to avoid style check errors in generated code.
(In reply to comment #4) > Committed r107182: <http://trac.webkit.org/changeset/107182> Reopen, because it broke many tests on Qt: http://build.webkit.org/results/Qt%20Linux%20Release/r107183%20%2843082%29/results.html Maybe on other platforms too, I didn't check them. Could you fix this regression? (In reply to comment #6) > (In reply to comment #4) > > Committed r107182: <http://trac.webkit.org/changeset/107182> > > Reopen, because it broke many tests on Qt: > http://build.webkit.org/results/Qt%20Linux%20Release/r107183%20%2843082%29/results.html > > Maybe on other platforms too, I didn't check them. Could you fix this regression? Fixed in r107191. Thanks. Reverted r107182 for reason: Layout tests of JSC-related port are crashing Committed r107204: <http://trac.webkit.org/changeset/107204> I rolled out the patch, as I couldn't fix the failure. (In reply to comment #0) > - Rename [Optional=CallWithDefalutValue] to [Optional=TreatAsUndefined] BTW, this should be renamed to [Optional=Undefined]. [Optional=TreatAsUndefined] is confusing with [TreatUndefinedAs=...]. (In reply to comment #0) > - Rename [Optional=CallWithDefalutValue] to [Optional=Undefined] > - Remove [Optional=CallWithNullValue]. Instead, we can use [Optional=Undefined, TreatUndefinedAs=NullString] I found that this renaming is wrong (this is the cause of build failures). If [Optional=Undefined, TreatUndefinedAs=NullString] is specified on a parameter, WebCore cannot distinguish the following two cases: - the parameter is the JavaScript undefined - the parameter is missing In other words, we cannot separately specify the behavior when the parameter is the JavaScript undefined and the behavior when the parameter is missing. Thus, my revised suggestion is - Rename [Optional=CallWithDefalutValue] to [Optional=Undefined] - Rename [Optional=CallWithNullValue] to [Optional=NullString] Created attachment 126279 [details]
Patch
Adam: Review? I confirmed that the uploaded patch passes layout tests which had been crashing in the previous patch. Attachment 126279 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:189: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:190: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:191: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 3 in 127 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 126279 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126279&action=review > Source/WebCore/ChangeLog:22 > + - Rename [Optional=CallWithDefalutValue] to [Optional=Undefined]. CallWithDefalutValue => CallWithDefaultValue Maybe: [Optional=CallWithDefalutValue] to [Optional=DefaultIsUndefined] [Optional=CallWithNullValue] to [Optional=DefaultIsNullString] ? [Optional=Undefined] is slightly mysterious. Created attachment 126397 [details]
Patch
(In reply to comment #15) > Maybe: > > [Optional=CallWithDefalutValue] to [Optional=DefaultIsUndefined] > [Optional=CallWithNullValue] to [Optional=DefaultIsNullString] Renamed as above. Attachment 126397 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:189: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:190: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h:191: The parameter name "str" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 3 in 141 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r107304: <http://trac.webkit.org/changeset/107304> |