patch for KJS "backwards for declarations" bug

Darin Adler darin at apple.com
Sat Oct 18 15:09:55 CEST 2003


This patch fixes a bug I introduced in JavaScriptCore last December 
when I changed it to use iteration rather than recursion to process 
lists. I missed one case where a list needs to be reversed because it's 
built backwards.

Pages that show this bug are 
<http://x.isomorphic.com/Safari_Tests/for_loop_jsError.html>, dragging 
images at <http://www.walterzorn.com>, and presumably many others.

-------------- next part --------------
Index: ChangeLog
===================================================================
RCS file: /local/home/cvs/Labyrinth/JavaScriptCore/ChangeLog,v
retrieving revision 1.362
diff -p -u -u -p -r1.362 ChangeLog
--- ChangeLog	2003/10/16 21:42:31	1.362
+++ ChangeLog	2003/10/18 21:05:29
@@ -1,3 +1,14 @@
+2003-10-18  Darin Adler  <darin at apple.com>
+
+        Reviewed by Dave.
+
+        - fixed 3367015 -- interdependent variable declarations in for loop don't work (they go backwards)
+
+        * kjs/nodes.h: (KJS::ForNode::ForNode): Add a new overload of the constructor for when the
+        first parameter is a variable declaration list. Call reverseList as we do in other constructors
+        that take lists that are built backwards.
+        * kjs/nodes.cpp: (ForNode::reverseList): Added. New helper function.
+
 === Safari-110 ===
 
 === Safari-109 ===
Index: kjs/nodes.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/JavaScriptCore/kjs/nodes.cpp,v
retrieving revision 1.42
diff -p -u -u -p -r1.42 kjs/nodes.cpp
--- kjs/nodes.cpp	2003/10/02 18:19:27	1.42
+++ kjs/nodes.cpp	2003/10/18 21:05:29
@@ -1987,6 +1987,18 @@ void WhileNode::processVarDecls(ExecStat
 
 // ------------------------------ ForNode --------------------------------------
 
+VarDeclListNode *ForNode::reverseList(VarDeclListNode *list)
+{
+  VarDeclListNode *head = 0;
+  VarDeclListNode *next;
+  for (VarDeclListNode *n = list; n; n = next) {
+    next = n->list;
+    n->list = head;
+    head = n;
+  }
+  return head;
+}
+
 void ForNode::ref()
 {
   Node::ref();
Index: kjs/nodes.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/JavaScriptCore/kjs/nodes.h,v
retrieving revision 1.15
diff -p -u -u -p -r1.15 kjs/nodes.h
--- kjs/nodes.h	2003/05/13 21:19:52	1.15
+++ kjs/nodes.h	2003/10/18 21:05:30
@@ -639,6 +639,7 @@ namespace KJS {
     virtual void processVarDecls(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
+    friend class ForNode;
     friend class VarStatementNode;
     VarDeclListNode *list;
     VarDeclNode *var;
@@ -732,12 +733,15 @@ namespace KJS {
   public:
     ForNode(Node *e1, Node *e2, Node *e3, StatementNode *s) :
       expr1(e1), expr2(e2), expr3(e3), statement(s) {}
+    ForNode(VarDeclListNode *e1, Node *e2, Node *e3, StatementNode *s) :
+      expr1(reverseList(e1)), expr2(e2), expr3(e3), statement(s) {}
     virtual void ref();
     virtual bool deref();
     virtual Completion execute(ExecState *exec);
     virtual void processVarDecls(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
+    static VarDeclListNode *reverseList(VarDeclListNode *);
     Node *expr1, *expr2, *expr3;
     StatementNode *statement;
   };
-------------- next part --------------


I remember Harri suggesting an even better way to build the lists that 
avoids having to reverse them, but I haven't made that change yet.

     -- Darin


More information about the Khtml-devel mailing list