Summary: | JSC GC uses dummy cells to avoid having to remember which cells it has already destroyed | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | fpizlo, oliver, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Filip Pizlo
2011-08-02 11:57:52 PDT
Created attachment 102674 [details]
the patch
Attachment 102674 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/heap/MarkedBlock.cpp:217: One line control clauses should not use braces. [whitespace/braces] [4]
Source/JavaScriptCore/heap/MarkedBlock.h:173: The parameter name "cell" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 2 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 102685 [details]
the patch
Comment on attachment 102685 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=102685&action=review > Source/JavaScriptCore/heap/MarkedBlock.h:77 > FreeCell* next; > + > + void setNoObject() > + { > + *reinterpret_cast<void**>(this) = 0; > + } Why this cast void**? FreeCell doesn't have a vtable pointer, you're simply assigning null to this->next? (In reply to comment #4) > (From update of attachment 102685 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=102685&action=review > > > Source/JavaScriptCore/heap/MarkedBlock.h:77 > > FreeCell* next; > > + > > + void setNoObject() > > + { > > + *reinterpret_cast<void**>(this) = 0; > > + } > > Why this cast void**? FreeCell doesn't have a vtable pointer, you're simply assigning null to this->next? I did that to emphasize the fact that I'm setting what-would-have-been-a-vtable to 0. I suppose a comment would do the same job, so I'll post a fix. Created attachment 102690 [details]
the patch
Comment on attachment 102690 [details] the patch Clearing flags on attachment: 102690 Committed r92233: <http://trac.webkit.org/changeset/92233> All reviewed patches have been landed. Closing bug. |