Summary: | Refactor binding generated casted-this attribute checks | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||
Component: | WebCore Misc. | Assignee: | youenn fablet <youennf> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | alecflett, beidson, cdumez, cgarcia, commit-queue, darin, jsbell | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
youenn fablet
2016-09-28 08:20:49 PDT
Created attachment 290084 [details]
WIP
Created attachment 290470 [details]
Improving style
(In reply to comment #0) > This would make the binding code more readable. > This might also allow in the future to disable this check for private > functions, where this is redundant since usually already done by the caller. This might also allow simplifying the handling of promise-returning methods. Comment on attachment 290470 [details] Improving style View in context: https://bugs.webkit.org/attachment.cgi?id=290470&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2851 > + } I tried here to stop mixing generator logic with generating code. I think it improves the readability to have such big chunks of outputted code without nested if/else. These chunks could later be moved to a template-dedicated file, like done for the built-in generator. This would allow to make the logic of the generator easier to understand. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2896 > + push(@implContent, "static inline JSValue ${getFunctionName}Getter(ExecState* state)\n"); > + push(@implContent, "{\n"); Ditto here. EFL and GTK build issues are unrelated to that patch, the bots were broken at the upload time. Comment on attachment 290470 [details] Improving style View in context: https://bugs.webkit.org/attachment.cgi?id=290470&action=review Change looks OK to me as long as it is performance neutral. Sadly this will conflict with my changes to CodeGeneratorJS.pm, so whoever goes second will need to merge. > Source/WebCore/ChangeLog:80 > + (WebCore::JSTestInterface::castForAttribute): > + (WebCore::jsTestInterfaceConstructorImplementsStaticReadOnlyAttr): > + (WebCore::jsTestInterfaceConstructorImplementsStaticReadOnlyAttrGetter): > + (WebCore::jsTestInterfaceConstructorImplementsStaticAttr): > + (WebCore::jsTestInterfaceConstructorImplementsStaticAttrGetter): > + (WebCore::jsTestInterfaceImplementsStr1): > + (WebCore::jsTestInterfaceImplementsStr1Getter): > + (WebCore::jsTestInterfaceImplementsStr2): > + (WebCore::jsTestInterfaceImplementsStr2Getter): > + (WebCore::jsTestInterfaceImplementsStr3): > + (WebCore::jsTestInterfaceImplementsStr3Getter): > + (WebCore::jsTestInterfaceImplementsNode): > + (WebCore::jsTestInterfaceImplementsNodeGetter): > + (WebCore::jsTestInterfaceConstructorSupplementalStaticReadOnlyAttr): > + (WebCore::jsTestInterfaceConstructorSupplementalStaticReadOnlyAttrGetter): > + (WebCore::jsTestInterfaceConstructorSupplementalStaticAttr): > + (WebCore::jsTestInterfaceConstructorSupplementalStaticAttrGetter): > + (WebCore::jsTestInterfaceSupplementalStr1): > + (WebCore::jsTestInterfaceSupplementalStr1Getter): > + (WebCore::jsTestInterfaceSupplementalStr2): > + (WebCore::jsTestInterfaceSupplementalStr2Getter): > + (WebCore::jsTestInterfaceSupplementalStr3): > + (WebCore::jsTestInterfaceSupplementalStr3Getter): > + (WebCore::jsTestInterfaceSupplementalNode): > + (WebCore::jsTestInterfaceSupplementalNodeGetter): These lists of function names are not useful for the bindings expected results. Please delete them to make the change log readable. Created attachment 291012 [details]
Patch for landing
Comment on attachment 291012 [details] Patch for landing Clearing flags on attachment: 291012 Committed r206953: <http://trac.webkit.org/changeset/206953> All reviewed patches have been landed. Closing bug. Thanks for the review. > Change looks OK to me as long as it is performance neutral. Sadly this will > conflict with my changes to CodeGeneratorJS.pm, so whoever goes second will > need to merge. Hope the merge will not be too much trouble. > > Source/WebCore/ChangeLog:80 > > + (WebCore::JSTestInterface::castForAttribute): > > + (WebCore::jsTestInterfaceConstructorImplementsStaticReadOnlyAttr): > > + (WebCore::jsTestInterfaceConstructorImplementsStaticReadOnlyAttrGetter): > > + (WebCore::jsTestInterfaceConstructorImplementsStaticAttr): > > + (WebCore::jsTestInterfaceConstructorImplementsStaticAttrGetter): > > + (WebCore::jsTestInterfaceImplementsStr1): > > + (WebCore::jsTestInterfaceImplementsStr1Getter): > > + (WebCore::jsTestInterfaceImplementsStr2): > > + (WebCore::jsTestInterfaceImplementsStr2Getter): > > + (WebCore::jsTestInterfaceImplementsStr3): > > + (WebCore::jsTestInterfaceImplementsStr3Getter): > > + (WebCore::jsTestInterfaceImplementsNode): > > + (WebCore::jsTestInterfaceImplementsNodeGetter): > > + (WebCore::jsTestInterfaceConstructorSupplementalStaticReadOnlyAttr): > > + (WebCore::jsTestInterfaceConstructorSupplementalStaticReadOnlyAttrGetter): > > + (WebCore::jsTestInterfaceConstructorSupplementalStaticAttr): > > + (WebCore::jsTestInterfaceConstructorSupplementalStaticAttrGetter): > > + (WebCore::jsTestInterfaceSupplementalStr1): > > + (WebCore::jsTestInterfaceSupplementalStr1Getter): > > + (WebCore::jsTestInterfaceSupplementalStr2): > > + (WebCore::jsTestInterfaceSupplementalStr2Getter): > > + (WebCore::jsTestInterfaceSupplementalStr3): > > + (WebCore::jsTestInterfaceSupplementalStr3Getter): > > + (WebCore::jsTestInterfaceSupplementalNode): > > + (WebCore::jsTestInterfaceSupplementalNodeGetter): > > These lists of function names are not useful for the bindings expected > results. Please delete them to make the change log readable. Done. |