WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED INVALID
12878
Support for each (var in collection) syntax
https://bugs.webkit.org/show_bug.cgi?id=12878
Summary
Support for each (var in collection) syntax
Justin Haygood
Reported
2007-02-24 10:50:20 PST
JavaScript 1.6 supports for each ( var in collection ) syntax, like: var array = ["foo","bar"]; for each ( var string in array ) { document.write( string + "<br>" ) } Should result in: foo<br> bar<br> Currently, it gets an error. This is one area where we can make the first effort at supporting E4X, but this is useful in any JavaScript/DOM collection as well.
Attachments
mini test case
(287 bytes, text/html)
2007-02-24 10:51 PST
,
Justin Haygood
no flags
Details
Adds support for for..each..in
(9.07 KB, patch)
2007-02-24 12:08 PST
,
Justin Haygood
ap
: review-
Details
Formatted Diff
Diff
Implements for each ( var <identifier> in <collection> )
(8.75 KB, patch)
2007-02-25 10:34 PST
,
Justin Haygood
mrowe
: review-
Details
Formatted Diff
Diff
Updated Patch
(8.10 KB, patch)
2007-08-01 09:43 PDT
,
Justin Haygood
mrowe
: review-
Details
Formatted Diff
Diff
for each...in test case
(1.60 KB, application/zip)
2007-08-01 13:15 PDT
,
Justin Haygood
no flags
Details
Updated Patch for for each...in
(8.00 KB, patch)
2007-08-01 13:18 PDT
,
Justin Haygood
mrowe
: review-
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Justin Haygood
Comment 1
2007-02-24 10:51:02 PST
Created
attachment 13362
[details]
mini test case Test case for bug. Works in Firefox, Fails elsewhere.
Justin Haygood
Comment 2
2007-02-24 12:08:06 PST
Created
attachment 13364
[details]
Adds support for for..each..in This is an initial implementation of for...each...in, supported by Mozilla Firefox currently. This also is required for support of E4X, but is useable elsewhere, such as iterating through arrays more naturally.
Alexey Proskuryakov
Comment 3
2007-02-24 12:41:05 PST
Comment on
attachment 13364
[details]
Adds support for for..each..in r- for purely stylistic issues discussed via IRC (star and comma positioning, assert vs. ASSERT). And clearly, this cannot be landed to trunk before the stabilization period ends. A couple additional stylistic notes: + if (varDecl) + s << "var " << varDecl; Indentation looks broken here. + if ((c.complType() == Break) && ls.contains(c.target())) + break; And here. + JSValue* e; ... + e = expr->evaluate(exec); There is no need to declare local variables at the beginning of the function in C++. Also, this and other variables may benefit from a more descriptive name.
Justin Haygood
Comment 4
2007-02-25 10:34:03 PST
Created
attachment 13368
[details]
Implements for each ( var <identifier> in <collection> ) Same as above mostly, except: * Removes unused variables * Follows code style guidelines
Mark Rowe (bdash)
Comment 5
2007-02-25 17:51:48 PST
Comment on
attachment 13368
[details]
Implements for each ( var <identifier> in <collection> ) +void ForEachInNode::streamTo(SourceStream &s) const +{ + s << SourceStream::Endl << "for each ("; + if (varDecl) + s << "var " << varDecl; + else + s << lexpr; + Indentation is incorrect here. + if ((c.complType() == Break) && ls.contains(c.target())) + break; And again here. The ForEachInNode constructor should set varDecl and lexpr in the initializer list, and the formatting of this list should match the coding style guidelines. It looks as though varDecl is only used in streamTo, which means that "for each (a in b)" and "for each (var a in b)" are treated identically when they should have different semantics. + JSValue* str = v->get(exec, name); "str" seems to be a bit misleading for the new use of this variable, I gather it's now the value of the property retrieved from the iterator. It seems that almost all of the code in ForEachInNode::execute is duplicated from ForInNode which is less than ideal. It's also not clear why supporting only a single feature from the E4X specification is desirable.
Mark Rowe (bdash)
Comment 6
2007-02-25 17:57:43 PST
Note that this feature comes from ECMA 357 (<
http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-357.pdf
>), the ECMAScript for XML specification. You should verify that your implementation matches this specification and definitely include test cases to support this.
Justin Haygood
Comment 7
2007-08-01 09:43:12 PDT
Created
attachment 15782
[details]
Updated Patch 1. Implemented by subclassing from ForInNode, so that it doesn't duplicate code effort 2. Testcases are on the way 3. Doesn't cause any regressions 4. This is an experimental patch. Questions/Concerns, etc.. are very much welcome!
Mark Rowe (bdash)
Comment 8
2007-08-01 10:25:04 PDT
Comment on
attachment 15782
[details]
Updated Patch I pointed out a few issues in the patch to Justin via IRC. There are numerous coding style inconsistencies, and the redeclaration of ForInNode's instance variables in the subclass is wrong. I also suggested that the implementation of streamTo could easily be shared between the two classes which should allow ForInNode's members to remain private.
Mark Rowe (bdash)
Comment 9
2007-08-01 10:25:56 PDT
Oh, and test cases! Test cases, test cases, test cases! ;-)
Justin Haygood
Comment 10
2007-08-01 13:15:42 PDT
Created
attachment 15790
[details]
for each...in test case Testcase
Justin Haygood
Comment 11
2007-08-01 13:18:24 PDT
Created
attachment 15791
[details]
Updated Patch for for each...in This has been modified to follow code style guidelines. It also merges streamTo between ForInNode and ForEachInNode and restores the "private" in ForInNode
Mark Rowe (bdash)
Comment 12
2007-08-01 20:32:50 PDT
Comment on
attachment 15791
[details]
Updated Patch for for each...in There is still incorrect whitespace and * placement in many places. You should also include a test case as part of the patch, along with the related changelog entry. + Node *n = $4->nodeInsideAllParens(); +JSValue* ForInNode::getIteratorValue( ExecState*, JSObject* inValue, const Identifier name ) + JSValue *str = getIteratorValue(exec,v,name); +JSValue* ForEachInNode::getIteratorValue(ExecState* exec, JSObject* inValue, const Identifier name ) + virtual JSValue* getIteratorValue( ExecState *exec, JSObject *inValue, const Identifier name ) KJS_FAST_CALL; + ForEachInNode(Node *l, Node *e, StatementNode *s) KJS_FAST_CALL; + ForEachInNode(const Identifier &i, AssignExprNode *in, Node *e, StatementNode *s) KJS_FAST_CALL; + protected: + virtual JSValue* getIteratorValue( ExecState *exec, JSObject *inValue, const Identifier name ) KJS_FAST_CALL;
Cameron Zwarich (cpst)
Comment 13
2008-09-08 09:13:06 PDT
***
Bug 20728
has been marked as a duplicate of this bug. ***
Gavin Barraclough
Comment 14
2012-03-08 18:11:54 PST
Our goal is ES-262 compliance, and I don't think it looks "for each" looks like it's going to make it into ES6. As such, I don't think we plan to support this. If "for each" makes it onto the standardization track, we should revisit this.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug