Bug 34282 – Simplify anonymous slot implementation

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Oliver Hunt]

Simplify anonymous slot implementation

[Oliver Hunt]

Created attachment 47646 [details]
Patch

[Gavin Barraclough]

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+.

[Darin Adler]

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

[Oliver Hunt]

Committed r54022: <http://trac.webkit.org/changeset/54022>

[Adam Barth]

This patch caused the qt-ews to continually fail to upload results to QueueStatusServer.  Not sure why.