Bug 9950

Summary: property name array changes
Product: WebKit Reporter: Maciej Stachowiak <mjs>
Component: JavaScriptCoreAssignee: Maciej Stachowiak <mjs>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, jesse, ytpete
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
JS property name fixes darin: review+

Maciej Stachowiak
Reported 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.
Attachments
JS property name fixes (80.66 KB, patch)
2006-07-16 04:55 PDT, Maciej Stachowiak 		darin: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Maciej Stachowiak
Comment 1 2006-07-16 04:55:15 PDT
Created attachment 9483 [details] JS property name fixes
Darin Adler
Comment 2 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
Darin Adler
Comment 3 2006-07-16 17:14:23 PDT
Maciej landed this as r15468.
Alexey Proskuryakov
Comment 4 2006-07-16 21:44:23 PDT
*** Bug 6639 has been marked as a duplicate of this bug. ***
Alexey Proskuryakov
Comment 5 2006-07-16 21:44:32 PDT
*** Bug 8742 has been marked as a duplicate of this bug. ***
Alexey Proskuryakov
Comment 6 2006-07-16 21:44:46 PDT
*** Bug 7543 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.