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
Adds support for for..each..in (9.07 KB, patch)
2007-02-24 12:08 PST, Justin Haygood
ap: review-
Implements for each ( var <identifier> in <collection> ) (8.75 KB, patch)
2007-02-25 10:34 PST, Justin Haygood
mrowe: review-
Updated Patch (8.10 KB, patch)
2007-08-01 09:43 PDT, Justin Haygood
mrowe: review-
for each...in test case (1.60 KB, application/zip)
2007-08-01 13:15 PDT, Justin Haygood
no flags
Updated Patch for for each...in (8.00 KB, patch)
2007-08-01 13:18 PDT, Justin Haygood
mrowe: review-
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.