Bug 34282 - Simplify anonymous slot implementation
Summary: Simplify anonymous slot implementation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-28 14:02 PST by Oliver Hunt
Modified: 2010-01-28 16:52 PST (History)
1 user (show)

See Also:


Attachments
Patch (44.43 KB, patch)
2010-01-28 14:10 PST, Oliver Hunt
barraclough: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2010-01-28 14:02:15 PST
Simplify anonymous slot implementation
Comment 1 Oliver Hunt 2010-01-28 14:10:05 PST
Created attachment 47646 [details]
Patch
Comment 2 Gavin Barraclough 2010-01-28 14:21:17 PST
Comment on attachment 47646 [details]
Patch

The hard coded ", 0" in JSCel / API wrappers is a little weird to me, I'd move AnonymousSlotCount up from JSValue personally.  Your call.  r+.
Comment 3 Darin Adler 2010-01-28 14:29:55 PST
Comment on attachment 47646 [details]
Patch

>  PassRefPtr<Structure> JSByteArray::createStructure(JSValue prototype)
>  {
> -    PassRefPtr<Structure> result = Structure::create(prototype, TypeInfo(ObjectType, StructureFlags));
> +    PassRefPtr<Structure> result = Structure::create(prototype, TypeInfo(ObjectType, StructureFlags), AnonymousSlotCount);
>      return result;
>  }

Why the local variable? Normally we don't use PassRefPtr for local variables, and normally we wouldn't use a local variable at all for this case.

> +        static const unsigned AnonymousSlotCount = 0;
>      protected:

It's dangerous to have something like this as a public data member. It gives the wrong value if you use it on a base class of an object of a derived class. Can we make it protected instead? Maybe we can name it InitialAnonymousSlotCount too.

>  Structure::~Structure()
>  {
>      if (m_previous) {
> -        if (m_nameInPrevious)
> -            m_previous->table.remove(make_pair(m_nameInPrevious.get(), m_attributesInPrevious), m_specificValueInPrevious);
> -        else
> -            m_previous->table.removeAnonymousSlotTransition(m_anonymousSlotsInPrevious);
> +        ASSERT(m_nameInPrevious);
> +        m_previous->table.remove(make_pair(m_nameInPrevious.get(), m_attributesInPrevious), m_specificValueInPrevious);
>  
>      }

You should remove that extra blank line.

r=me
Comment 4 Oliver Hunt 2010-01-28 14:51:18 PST
Committed r54022: <http://trac.webkit.org/changeset/54022>
Comment 5 Adam Barth 2010-01-28 16:52:23 PST
This patch caused the qt-ews to continually fail to upload results to QueueStatusServer.  Not sure why.