Summary: | Merge StructureShapes that share the same prototype chain | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||
Component: | JavaScriptCore | Assignee: | Saam Barati <saam> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, fpizlo | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Saam Barati
2014-09-04 14:48:06 PDT
(In reply to comment #0) > Create a more sophisticated merge of StructureShapes that will merge objects with the same prototype chain. And also create the notion of optional fields. Created attachment 237674 [details]
patch
Attachment 237674 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/TypeSet.cpp:86: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4]
ERROR: Source/JavaScriptCore/runtime/TypeSet.cpp:535: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5]
ERROR: Source/JavaScriptCore/runtime/TypeSet.h:81: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/TypeSet.h:82: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5]
Total errors found: 4 in 4 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 237675 [details]
patch
Comment on attachment 237675 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=237675&action=review > Source/JavaScriptCore/runtime/TypeSet.cpp:69 > +void TypeSet::addTypeInformation(RuntimeType type, PassRefPtr<StructureShape> prpNewShape, StructureID id) What's is "prp"? We usually stay away from abbreviations in variable names. > Source/JavaScriptCore/runtime/TypeSet.cpp:557 > + for (auto iter = a->m_fields.begin(), end = a->m_fields.end(); iter != end; ++iter) { Any reason why these loops aren't C++11 for-in loops, like: for (auto value : a->m_fields) ...? (In reply to comment #5) > (From update of attachment 237675 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=237675&action=review > > > Source/JavaScriptCore/runtime/TypeSet.cpp:69 > > +void TypeSet::addTypeInformation(RuntimeType type, PassRefPtr<StructureShape> prpNewShape, StructureID id) > > What's is "prp"? We usually stay away from abbreviations in variable names. I think it stands for pass reference pointer. That's what the webkit article says to prefix variables with that are PassRefPtr but immediately put into RefPtrs with. Should I switch to something else? Usually how I like to handle this is: function (_foo) { var foo = _foo }; > > > Source/JavaScriptCore/runtime/TypeSet.cpp:557 > > + for (auto iter = a->m_fields.begin(), end = a->m_fields.end(); iter != end; ++iter) { > > Any reason why these loops aren't C++11 for-in loops, like: for (auto value : a->m_fields) ...? No, I should switch to that. Created attachment 237695 [details]
patch
landed in: http://trac.webkit.org/changeset/173388 |