Bug 3999 - Object.prototype is missing propertyIsEnumerable
Summary: Object.prototype is missing propertyIsEnumerable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 412
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-07-14 10:41 PDT by Geoffrey Garen
Modified: 2005-12-04 15:31 PST (History)
0 users

See Also:


Attachments
Implement propertyIsEnumerable (9.30 KB, patch)
2005-12-04 13:32 PST, Anders Carlsson
ggaren: review-
Details | Formatted Diff | Diff
Address comments (10.78 KB, patch)
2005-12-04 14:54 PST, Anders Carlsson
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2005-07-14 10:41:02 PDT
Section 15.2.4 of ECMA-262
Comment 1 Anders Carlsson 2005-12-04 13:32:08 PST
Created attachment 4942 [details]
Implement propertyIsEnumerable
Comment 2 Geoffrey Garen 2005-12-04 13:42:06 PST
Comment on attachment 4942 [details]
Implement propertyIsEnumerable

isPropertyEnumerable should just call ObjectImp::get instead of doing the
lookup manually.

The putDirect calls need spaces after the commas, per our style guidelines.

r- because of the above.

I think it would be clearer if you named the C++ function propertyIsEnumerable
instead of isPropertyEnumerable, to match the name of the corresponding
(admittedly poorly named) JS function. I'll leave that decision up to you.
Comment 3 Anders Carlsson 2005-12-04 13:47:50 PST
(In reply to comment #2)
> (From update of attachment 4942 [details] [edit])
> isPropertyEnumerable should just call ObjectImp::get instead of doing the
> lookup manually.
> 
The problem is that ObjectImp::get doesn't return the property flags. ObjectImp::canPut does the same 
thing in order to get the flags.

> The putDirect calls need spaces after the commas, per our style guidelines.
OK. I was just following the style that was in the file.

> 
> r- because of the above.
> 
> I think it would be clearer if you named the C++ function propertyIsEnumerable
> instead of isPropertyEnumerable, to match the name of the corresponding
> (admittedly poorly named) JS function. I'll leave that decision up to you.
> 
Sounds like a good idea.
Comment 4 Anders Carlsson 2005-12-04 14:54:53 PST
Created attachment 4949 [details]
Address comments
Comment 5 Geoffrey Garen 2005-12-04 15:18:48 PST
Comment on attachment 4949 [details]
Address comments

ggaren: andersca:
[3:10pm] ggaren: +    return !(attributes & DontEnum);
[3:10pm] ggaren: is backwards, no?
[3:12pm] ggaren: +    putDirect(valueOfPropertyName,  new
ObjectProtoFuncImp(exec, funcProto, ObjectProtoFuncImp::ValueOf,	       
0), DontEnum);
[3:12pm] ggaren: has an extra space

Otherwise, looks great.
Comment 6 Geoffrey Garen 2005-12-04 15:26:35 PST
Comment on attachment 4949 [details]
Address comments

My bad. Not Don't Enum == enumberable. 

r=me