[webkit-reviews] review granted: [Bug 33731] [Dupe of] Many false leaks in release builds due to PtrAndFlags : [Attachment 48735] Final Patch! Refactor StructureTransitionTable back into Structure, remove PtrAndFlags!

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Feb 15 12:54:47 PST 2010


Darin Adler <darin at apple.com> has granted Gavin Barraclough
<barraclough at apple.com>'s request for review:
Bug 33731: [Dupe of] Many false leaks in release builds due to PtrAndFlags
https://bugs.webkit.org/show_bug.cgi?id=33731

Attachment 48735: Final Patch!	Refactor StructureTransitionTable back into
Structure, remove PtrAndFlags!
https://bugs.webkit.org/attachment.cgi?id=48735&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    , m_usingSingleSlot(true)

The naming scheme for booleans where we make them finish a sentence "structure
<xxx>" would mean this should be named m_isUsingSingleSlot.

> +	   bool structureTransitionTableContains(const
StructureTransitionTableHash::Key& key, JSCell* specificValue)
> +	   Structure* structureTransitionTableGet(const
StructureTransitionTableHash::Key& key, JSCell* specificValue) const
> +	   bool structureTransitionTableHasTransition(const
StructureTransitionTableHash::Key& key) const
> +	   void structureTransitionTableRemove(const
StructureTransitionTableHash::Key& key, JSCell* specificValue)
> +	   void structureTransitionTableAdd(const
StructureTransitionTableHash::Key& key, Structure* structure, JSCell*
specificValue)
> +	   TransitionTable* structureTransitionTable() const {
ASSERT(!m_usingSingleSlot); return m_transitions.m_table; }
> +	   void setStructureTransitionTable(TransitionTable* table)

Seems we should name these function members transitionTableXXX since they are
members of the Structure class.

It also seems like the header would be a little easier to read if the function
bodies were outside the class definition, marked inline. Definitions of
functions that are used only inside Structure.cpp can go in that file rather
than the header.

r=me


More information about the webkit-reviews mailing list