Bug 9950 - property name array changes
Summary: property name array changes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Maciej Stachowiak
URL:
Keywords:
: 6639 7543 8742 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-07-16 04:54 PDT by Maciej Stachowiak
Modified: 2006-07-16 21:44 PDT (History)
3 users (show)

See Also:


Attachments
JS property name fixes (80.66 KB, patch)
2006-07-16 04:55 PDT, Maciej Stachowiak
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 2006-07-16 04:54:38 PDT
A bunch of changes to JavaScriptCore to remove the Reference type, simplify and clean up public API for property names, and avoid duplicate property names.
Comment 1 Maciej Stachowiak 2006-07-16 04:55:15 PDT
Created attachment 9483 [details]
JS property name fixes
Comment 2 Darin Adler 2006-07-16 05:09:04 PDT
Comment on attachment 9483 [details]
JS property name fixes

+        PropertyNameArray() {}

This line of code is not needed.

+        void deallocateVector();

This declaration is not needed.

+        typedef HashSet<UString::Impl*, PtrHash<UString::Impl*> > IdentifierSet;

Why do you specify PtrHash explicitly? Isn't that the default?

+    virtual void getPropertyNames(ExecState *exec, PropertyNameArray& propertyNames);

I'd write the above as:

    virtual void getPropertyNames(ExecState*, PropertyNameArray&);

I can tell that you want to renaming UString::Rep to UString::Impl -- we should do that! Not sure why you wanted to create the synonym just for this patch though.

+"This tests that for/in statements don't report properties that are in both an object ant its prototype more than once."

Lets say "and" rather than "ant" here.

+ *  Copyright (C) 2005 Apple Computer, Inc

Should be 2006.

+#ifndef KJS_IDENTIFIER_SEQUENCED_SET_H

Since you chose to call this PropertyNameArray, I recommend making the header guard say that too.

I think JSPropertyNameArray is a fine name for the API. But internally, maybe this should be JSPropertyNameVector. I know it's a dual vector/set, but it can also be thought of as a vector with a special "add" operation. The use of array in the API is really just to be consistent with CFArray, and our C++ equivalent of CFArray is Vector.

r=me
Comment 3 Darin Adler 2006-07-16 17:14:23 PDT
Maciej landed this as r15468.
Comment 4 Alexey Proskuryakov 2006-07-16 21:44:23 PDT
*** Bug 6639 has been marked as a duplicate of this bug. ***
Comment 5 Alexey Proskuryakov 2006-07-16 21:44:32 PDT
*** Bug 8742 has been marked as a duplicate of this bug. ***
Comment 6 Alexey Proskuryakov 2006-07-16 21:44:46 PDT
*** Bug 7543 has been marked as a duplicate of this bug. ***