Node::reverse*() cleanup

Darin Adler darin at apple.com
Sun Nov 2 11:01:31 CET 2003


I spent a few minutes to look at this. I haven't landed a patch in our 
tree yet, but I have one I think is ready; my first try failed some 
tests, but it was due to a mistake I made merging.

On Oct 28, 2003, at 4:56 PM, porten at froglogic.com wrote:

> +    CaseClauseNode(Node *e, StatListNode *l)
> +      : expr(e) { if (l) { list = l; l->list = 0; } else { list = 0; 
> } }

The above code looks wrong to me. I think it should be list = l->list, 
not list = l. This indicates that there aren't sufficient tests for 
this; I believe the symptom of the bug would be that any case with more 
than one statement would execute only the last statement, plus a 
storage leak.

I applied the "leave out the parameter to separate out the 0 case" 
optimization to the above CaseClauseNode constructor, one place where 
you did not do it.

The patch didn't include ObjectLiteralNode or ArgumentListNode, but I 
guess that's because you did that in an earlier patch. I was slightly 
surprised by that. I had expected to see it all at once; I didn't 
realize you had already landed part of this.

Here's the patch I have so far; I figured you might want to see my 
version.

-------------- next part --------------
Index: kjs/grammar.y
===================================================================
RCS file: /local/home/cvs/Labyrinth/JavaScriptCore/kjs/grammar.y,v
retrieving revision 1.15
diff -p -u -u -p -r1.15 kjs/grammar.y
--- kjs/grammar.y	2003/10/30 18:42:38	1.15
+++ kjs/grammar.y	2003/11/02 09:54:38
@@ -77,6 +77,8 @@ using namespace KJS;
   Operator            op;
   PropertyValueNode   *plist;
   PropertyNode        *pnode;
+  CatchNode           *cnode;
+  FinallyNode         *fnode;
 }
 
 %start Program
@@ -130,8 +132,10 @@ using namespace KJS;
 %type <node>  ConditionalExpr AssignmentExpr
 %type <node>  ExprOpt
 %type <node>  CallExpr
-%type <node>  Catch Finally
 
+%type <cnode> Catch
+%type <fnode> Finally
+
 %type <stat>  Statement Block
 %type <stat>  VariableStatement EmptyStatement ExprStatement
 %type <stat>  IfStatement IterationStatement ContinueStatement
@@ -183,7 +187,7 @@ PrimaryExpr:
   | Literal
   | ArrayLiteral
   | '(' Expr ')'                   { $$ = new GroupNode($2); }
-  | '{' '}'                        { $$ = new ObjectLiteralNode(0L); }
+  | '{' '}'                        { $$ = new ObjectLiteralNode(); }
   | '{' PropertyNameAndValueList '}'   { $$ = new ObjectLiteralNode($2); }
 ;
 
@@ -242,7 +246,7 @@ CallExpr:
 ;
 
 Arguments:
-    '(' ')'                        { $$ = new ArgumentsNode(0L); }
+    '(' ')'                        { $$ = new ArgumentsNode(); }
   | '(' ArgumentList ')'           { $$ = new ArgumentsNode($2); }
 ;
 
@@ -398,7 +402,7 @@ Statement:
 ;
 
 Block:
-    '{' '}'                        { $$ = new BlockNode(0L); DBG($$, @2, @2); }
+    '{' '}'                        { $$ = new BlockNode(0); DBG($$, @2, @2); }
   | '{' SourceElements '}'          { $$ = new BlockNode($2); DBG($$, @3, @3); }
 ;
 
@@ -449,7 +453,7 @@ ExprStatement:
 ;
 
 IfStatement: /* shift/reduce conflict due to dangling else */
-    IF '(' Expr ')' Statement      { $$ = new IfNode($3,$5,0L);DBG($$, at 1, at 4); }
+    IF '(' Expr ')' Statement      { $$ = new IfNode($3,$5,0);DBG($$, at 1, at 4); }
   | IF '(' Expr ')' Statement ELSE Statement
                                    { $$ = new IfNode($3,$5,$7);DBG($$, at 1, at 4); }
 ;
@@ -467,7 +471,7 @@ IterationStatement:
             Statement              { $$ = new ForInNode($3, $5, $7);
 	                             DBG($$, at 1, at 6); }
   | FOR '(' VAR IDENT IN Expr ')'
-            Statement              { $$ = new ForInNode(*$4,0L,$6,$8);
+            Statement              { $$ = new ForInNode(*$4,0,$6,$8);
 	                             DBG($$, at 1, at 7); }
   | FOR '(' VAR IDENT Initializer IN Expr ')'
             Statement              { $$ = new ForInNode(*$4,$5,$7,$9);
@@ -475,7 +479,7 @@ IterationStatement:
 ;
 
 ExprOpt:
-    /* nothing */                  { $$ = 0L; }
+    /* nothing */                  { $$ = 0; }
   | Expr
 ;
 
@@ -507,9 +511,9 @@ BreakStatement:
 ;
 
 ReturnStatement:
-    RETURN ';'                     { $$ = new ReturnNode(0L); DBG($$, at 1, at 2); }
+    RETURN ';'                     { $$ = new ReturnNode(0); DBG($$, at 1, at 2); }
   | RETURN error                   { if (automatic()) {
-                                       $$ = new ReturnNode(0L); DBG($$, at 1, at 1);
+                                       $$ = new ReturnNode(0); DBG($$, at 1, at 1);
                                      } else
 				       YYABORT; }
   | RETURN Expr ';'                { $$ = new ReturnNode($2); }
@@ -530,13 +534,13 @@ SwitchStatement:
 ;
 
 CaseBlock:
-    '{' CaseClausesOpt '}'         { $$ = new CaseBlockNode($2, 0L, 0L); }
+    '{' CaseClausesOpt '}'         { $$ = new CaseBlockNode($2, 0, 0); }
   | '{' CaseClausesOpt DefaultClause CaseClausesOpt '}'
                                    { $$ = new CaseBlockNode($2, $3, $4); }
 ;
 
 CaseClausesOpt:
-    /* nothing */                  { $$ = 0L; }
+    /* nothing */                  { $$ = 0; }
   | CaseClauses
 ;
 
@@ -546,13 +550,13 @@ CaseClauses:
 ;
 
 CaseClause:
-    CASE Expr ':'                  { $$ = new CaseClauseNode($2, 0L); }
+    CASE Expr ':'                  { $$ = new CaseClauseNode($2); }
   | CASE Expr ':' StatementList    { $$ = new CaseClauseNode($2, $4); }
 ;
 
 DefaultClause:
-    DEFAULT ':'                    { $$ = new CaseClauseNode(0L, 0L);; }
-  | DEFAULT ':' StatementList      { $$ = new CaseClauseNode(0L, $3); }
+    DEFAULT ':'                    { $$ = new CaseClauseNode(0); }
+  | DEFAULT ':' StatementList      { $$ = new CaseClauseNode(0, $3); }
 ;
 
 LabelledStatement:
@@ -566,7 +570,7 @@ ThrowStatement:
 
 TryStatement:
     TRY Block Catch                { $$ = new TryNode($2, $3); }
-  | TRY Block Finally              { $$ = new TryNode($2, 0L, $3); }
+  | TRY Block Finally              { $$ = new TryNode($2, $3); }
   | TRY Block Catch Finally        { $$ = new TryNode($2, $3, $4); }
 ;
 
@@ -579,12 +583,12 @@ Finally:
 ;
 
 FunctionDeclaration:
-    FUNCTION IDENT '(' ')' FunctionBody    { $$ = new FuncDeclNode(*$2, 0L, $5); }
+    FUNCTION IDENT '(' ')' FunctionBody    { $$ = new FuncDeclNode(*$2, $5); }
   | FUNCTION IDENT '(' FormalParameterList ')' FunctionBody
                                    { $$ = new FuncDeclNode(*$2, $4, $6); }
 
 FunctionExpr:
-    FUNCTION '(' ')' FunctionBody  { $$ = new FuncExprNode(0L, $4); }
+    FUNCTION '(' ')' FunctionBody  { $$ = new FuncExprNode($4); }
   | FUNCTION '(' FormalParameterList ')' FunctionBody
                                    { $$ = new FuncExprNode($3, $5); }
 
@@ -596,14 +600,14 @@ FormalParameterList:
 ;
 
 FunctionBody:
-    '{' '}'  /* TODO: spec ??? */  { $$ = new FunctionBodyNode(0L);
+    '{' '}'  /* TODO: spec ??? */  { $$ = new FunctionBodyNode(0);
 	                             DBG($$, @1, @2);}
   | '{' SourceElements '}'         { $$ = new FunctionBodyNode($2);
 	                             DBG($$, @1, @3);}
 ;
 
 Program:
-    /* nothing, empty script */      { $$ = new ProgramNode(0L);
+    /* nothing, empty script */      { $$ = new ProgramNode(0);
                                      Parser::progNode = $$; }
     | SourceElements                 { $$ = new ProgramNode($1);
                                      Parser::progNode = $$; }
Index: kjs/nodes.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/JavaScriptCore/kjs/nodes.cpp,v
retrieving revision 1.44
diff -p -u -u -p -r1.44 kjs/nodes.cpp
--- kjs/nodes.cpp	2003/10/28 22:52:29	1.44
+++ kjs/nodes.cpp	2003/11/02 09:54:39
@@ -341,18 +341,6 @@ Value ElementNode::evaluate(ExecState *e
 
 // ------------------------------ ArrayNode ------------------------------------
 
-void ArrayNode::reverseElementList()
-{
-  ElementNode *head = 0;
-  ElementNode *next;
-  for (ElementNode *n = element; n; n = next) {
-    next = n->list;
-    n->list = head;
-    head = n;
-  }
-  element = head;
-}
-
 void ArrayNode::ref()
 {
   Node::ref();
@@ -391,18 +379,6 @@ Value ArrayNode::evaluate(ExecState *exe
 
 // ------------------------------ ObjectLiteralNode ----------------------------
 
-void ObjectLiteralNode::reverseList()
-{
-  PropertyValueNode *head = 0;
-  PropertyValueNode *next;
-  for (PropertyValueNode *n = list; n; n = next) {
-    next = n->list;
-    n->list = head;
-    head = n;
-  }
-  list = head;
-}
-
 void ObjectLiteralNode::ref()
 {
   Node::ref();
@@ -560,15 +536,6 @@ Reference AccessorNode2::evaluateReferen
 
 // ------------------------------ ArgumentListNode -----------------------------
 
-ArgumentListNode::ArgumentListNode(Node *e) : list(0L), expr(e)
-{
-}
-
-ArgumentListNode::ArgumentListNode(ArgumentListNode *l, Node *e)
-  : list(l), expr(e)
-{
-}
-
 void ArgumentListNode::ref()
 {
   for (ArgumentListNode *n = this; n; n = n->list) {
@@ -613,18 +580,6 @@ List ArgumentListNode::evaluateList(Exec
 
 // ------------------------------ ArgumentsNode --------------------------------
 
-void ArgumentsNode::reverseList()
-{
-  ArgumentListNode *head = 0;
-  ArgumentListNode *next;
-  for (ArgumentListNode *n = list; n; n = next) {
-    next = n->list;
-    n->list = head;
-    head = n;
-  }
-  list = head;
-}
-
 void ArgumentsNode::ref()
 {
   Node::ref();
@@ -1512,6 +1467,19 @@ Value CommaNode::evaluate(ExecState *exe
 
 // ------------------------------ StatListNode ---------------------------------
 
+StatListNode::StatListNode(StatementNode *s)
+  : statement(s), list(this)
+{
+  setLoc(s->firstLine(), s->lastLine(), s->sourceId());
+}
+ 
+StatListNode::StatListNode(StatListNode *l, StatementNode *s)
+  : statement(s), list(l->list)
+{
+  l->list = this;
+  setLoc(l->firstLine(), s->lastLine(), l->sourceId());
+}
+
 void StatListNode::ref()
 {
   for (StatListNode *n = this; n; n = n->list) {
@@ -1700,18 +1668,6 @@ void VarDeclListNode::processVarDecls(Ex
 
 // ------------------------------ VarStatementNode -----------------------------
 
-void VarStatementNode::reverseList()
-{
-  VarDeclListNode *head = 0;
-  VarDeclListNode *next;
-  for (VarDeclListNode *n = list; n; n = next) {
-    next = n->list;
-    n->list = head;
-    head = n;
-  }
-  list = head;
-}
-
 void VarStatementNode::ref()
 {
   Node::ref();
@@ -1744,16 +1700,15 @@ void VarStatementNode::processVarDecls(E
 
 // ------------------------------ BlockNode ------------------------------------
 
-void BlockNode::reverseList()
+BlockNode::BlockNode(SourceElementsNode *s)
 {
-  SourceElementsNode *head = 0;
-  SourceElementsNode *next;
-  for (SourceElementsNode *n = source; n; n = next) {
-    next = n->elements;
-    n->elements = head;
-    head = n;
+  if (s) {
+    source = s->elements;
+    s->elements = 0;
+    setLoc(s->firstLine(), s->lastLine(), s->sourceId());
+  } else {
+    source = 0;
   }
-  source = head;
 }
 
 void BlockNode::ref()
@@ -1988,18 +1943,6 @@ 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();
@@ -2274,18 +2217,6 @@ void WithNode::processVarDecls(ExecState
 
 // ------------------------------ CaseClauseNode -------------------------------
 
-void CaseClauseNode::reverseList()
-{
-  StatListNode *head = 0;
-  StatListNode *next;
-  for (StatListNode *n = list; n; n = next) {
-    next = n->list;
-    n->list = head;
-    head = n;
-  }
-  list = head;
-}
-
 void CaseClauseNode::ref()
 {
   Node::ref();
@@ -2369,26 +2300,26 @@ void ClauseListNode::processVarDecls(Exe
 
 // ------------------------------ CaseBlockNode --------------------------------
 
-void CaseBlockNode::reverseLists()
+CaseBlockNode::CaseBlockNode(ClauseListNode *l1, CaseClauseNode *d,
+                             ClauseListNode *l2)
 {
-  ClauseListNode *head = 0;
-  ClauseListNode *next;
-  for (ClauseListNode *n = list1; n; n = next) {
-    next = n->nx;
-    n->nx = head;
-    head = n;
+  if (l1) {
+    list1 = l1->nx;
+    l1->nx = 0;
+  } else {
+    list1 = 0;
   }
-  list1 = head;
-  
-  head = 0;
-  for (ClauseListNode *n = list2; n; n = next) {
-    next = n->nx;
-    n->nx = head;
-    head = n;
+
+  def = d;
+
+  if (l2) {
+    list2 = l2->nx;
+    l2->nx = 0;
+  } else {
+    list2 = 0;
   }
-  list2 = head;
 }
-
+ 
 void CaseBlockNode::ref()
 {
   Node::ref();
@@ -2772,18 +2703,6 @@ void FunctionBodyNode::processFuncDecl(E
 
 // ------------------------------ FuncDeclNode ---------------------------------
 
-void FuncDeclNode::reverseParameterList()
-{
-  ParameterNode *head = 0;
-  ParameterNode *next;
-  for (ParameterNode *n = param; n; n = next) {
-    next = n->next;
-    n->next = head;
-    head = n;
-  }
-  param = head;
-}
-
 void FuncDeclNode::ref()
 {
   Node::ref();
@@ -2837,18 +2756,6 @@ void FuncDeclNode::processFuncDecl(ExecS
 
 // ------------------------------ FuncExprNode ---------------------------------
 
-void FuncExprNode::reverseParameterList()
-{
-  ParameterNode *head = 0;
-  ParameterNode *next;
-  for (ParameterNode *n = param; n; n = next) {
-    next = n->next;
-    n->next = head;
-    head = n;
-  }
-  param = head;
-}
-
 void FuncExprNode::ref()
 {
   Node::ref();
@@ -2885,6 +2792,19 @@ Value FuncExprNode::evaluate(ExecState *
 }
 
 // ------------------------------ SourceElementsNode ---------------------------
+
+SourceElementsNode::SourceElementsNode(StatementNode *s1)
+  : element(s1), elements(this)
+{
+  setLoc(s1->firstLine(), s1->lastLine(), s1->sourceId());
+}
+ 
+SourceElementsNode::SourceElementsNode(SourceElementsNode *s1, StatementNode *s2)
+  : element(s2), elements(s1->elements)
+{
+  s1->elements = this;
+  setLoc(s1->firstLine(), s2->lastLine(), s1->sourceId());
+}
 
 void SourceElementsNode::ref()
 {
Index: kjs/nodes.h
===================================================================
RCS file: /local/home/cvs/Labyrinth/JavaScriptCore/kjs/nodes.h,v
retrieving revision 1.16
diff -p -u -u -p -r1.16 kjs/nodes.h
--- kjs/nodes.h	2003/10/18 21:13:32	1.16
+++ kjs/nodes.h	2003/11/02 09:54:39
@@ -209,9 +209,10 @@ namespace KJS {
 
   class ElementNode : public Node {
   public:
-    ElementNode(int e, Node *n) : list(0L), elision(e), node(n) { }
+    // list is circular until cracked in ArrayNode ctor
+    ElementNode(int e, Node *n) : list(this), elision(e), node(n) { }
     ElementNode(ElementNode *l, int e, Node *n)
-      : list(l), elision(e), node(n) { }
+      : list(l->list), elision(e), node(n) { l->list = this; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
@@ -225,46 +226,48 @@ namespace KJS {
 
   class ArrayNode : public Node {
   public:
-    ArrayNode(int e) : element(0L), elision(e), opt(true) { }
+    ArrayNode(int e) : element(0), elision(e), opt(true) { }
     ArrayNode(ElementNode *ele)
-      : element(ele), elision(0), opt(false) { reverseElementList(); }
+      : element(ele->list), elision(0), opt(false) { ele->list = 0; }
     ArrayNode(int eli, ElementNode *ele)
-      : element(ele), elision(eli), opt(true) { reverseElementList(); }
+      : element(ele->list), elision(eli), opt(true) { ele->list = 0; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
-    void reverseElementList();
     ElementNode *element;
     int elision;
     bool opt;
   };
 
-  class ObjectLiteralNode : public Node {
+  class PropertyValueNode : public Node {
   public:
-    ObjectLiteralNode(PropertyValueNode *l) : list(l) { reverseList(); }
+    // list is circular until cracked in ObjectLiteralNode ctor
+    PropertyValueNode(PropertyNode *n, Node *a)
+      : name(n), assign(a), list(this) { }
+    PropertyValueNode(PropertyNode *n, Node *a, PropertyValueNode *l)
+      : name(n), assign(a), list(l->list) { l->list = this; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
-    void reverseList();
+    friend class ObjectLiteralNode;
+    PropertyNode *name;
+    Node *assign;
     PropertyValueNode *list;
   };
 
-  class PropertyValueNode : public Node {
+  class ObjectLiteralNode : public Node {
   public:
-    PropertyValueNode(PropertyNode *n, Node *a, PropertyValueNode *l = 0L)
-      : name(n), assign(a), list(l) { }
+    ObjectLiteralNode() : list(0) { }
+    ObjectLiteralNode(PropertyValueNode *l) : list(l->list) { l->list = 0; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
-    friend class ObjectLiteralNode;
-    PropertyNode *name;
-    Node *assign;
     PropertyValueNode *list;
   };
 
@@ -307,8 +310,10 @@ namespace KJS {
 
   class ArgumentListNode : public Node {
   public:
-    ArgumentListNode(Node *e);
-    ArgumentListNode(ArgumentListNode *l, Node *e);
+    // list is circular until cracked in ArgumentsNode ctor
+    ArgumentListNode(Node *e) : list(this), expr(e) { }
+    ArgumentListNode(ArgumentListNode *l, Node *e)
+      : list(l->list), expr(e) { l->list = this; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
@@ -322,20 +327,21 @@ namespace KJS {
 
   class ArgumentsNode : public Node {
   public:
-    ArgumentsNode(ArgumentListNode *l) : list(l) { reverseList(); }
+    ArgumentsNode() : list(0) { }
+    ArgumentsNode(ArgumentListNode *l)
+      : list(l->list) { l->list = 0; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
     List evaluateList(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
-    void reverseList();
     ArgumentListNode *list;
   };
 
   class NewExprNode : public Node {
   public:
-    NewExprNode(Node *e) : expr(e), args(0L) {}
+    NewExprNode(Node *e) : expr(e), args(0) {}
     NewExprNode(Node *e, ArgumentsNode *a) : expr(e), args(a) {}
     virtual void ref();
     virtual bool deref();
@@ -592,8 +598,9 @@ namespace KJS {
 
   class StatListNode : public StatementNode {
   public:
-    StatListNode(StatementNode *s) : statement(s), list(0L) { }
-    StatListNode(StatListNode *l, StatementNode *s) : statement(s), list(l) { }
+    // list is circular until cracked in CaseClauseNode ctor
+    StatListNode(StatementNode *s);
+    StatListNode(StatListNode *l, StatementNode *s);
     virtual void ref();
     virtual bool deref();
     virtual Completion execute(ExecState *exec);
@@ -631,8 +638,10 @@ namespace KJS {
 
   class VarDeclListNode : public Node {
   public:
-    VarDeclListNode(VarDeclNode *v) : list(0L), var(v) {}
-    VarDeclListNode(VarDeclListNode *l, VarDeclNode *v) : list(l), var(v) {}
+    // list is circular until cracked in VarStatementNode/ForNode ctor
+    VarDeclListNode(VarDeclNode *v) : list(this), var(v) {}
+    VarDeclListNode(VarDeclListNode *l, VarDeclNode *v)
+      : list(l->list), var(v) { l->list = this; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
@@ -647,27 +656,26 @@ namespace KJS {
 
   class VarStatementNode : public StatementNode {
   public:
-    VarStatementNode(VarDeclListNode *l) : list(l) { reverseList(); }
+    VarStatementNode(VarDeclListNode *l)
+      : list(l->list) { l->list = 0; }
     virtual void ref();
     virtual bool deref();
     virtual Completion execute(ExecState *exec);
     virtual void processVarDecls(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
-    void reverseList();
     VarDeclListNode *list;
   };
 
   class BlockNode : public StatementNode {
   public:
-    BlockNode(SourceElementsNode *s) : source(s) { reverseList(); }
+    BlockNode(SourceElementsNode *s);
     virtual void ref();
     virtual bool deref();
     virtual Completion execute(ExecState *exec);
     virtual void processVarDecls(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   protected:
-    void reverseList();
     SourceElementsNode *source;
   };
 
@@ -734,14 +742,13 @@ namespace KJS {
     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) {}
+      expr1(e1->list), expr2(e2), expr3(e3), statement(s) { e1->list = 0; }
     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;
   };
@@ -809,7 +816,9 @@ namespace KJS {
 
   class CaseClauseNode: public Node {
   public:
-    CaseClauseNode(Node *e, StatListNode *l) : expr(e), list(l) { reverseList(); }
+    CaseClauseNode(Node *e) : expr(e), list(0) { }
+    CaseClauseNode(Node *e, StatListNode *l)
+      : expr(e), list(l->list) { l->list = 0; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
@@ -817,15 +826,15 @@ namespace KJS {
     virtual void processVarDecls(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
-    void reverseList();
     Node *expr;
     StatListNode *list;
   };
 
   class ClauseListNode : public Node {
   public:
-    ClauseListNode(CaseClauseNode *c) : cl(c), nx(0L) { }
-    ClauseListNode(ClauseListNode *n, CaseClauseNode *c) : cl(c), nx(n) { }
+    ClauseListNode(CaseClauseNode *c) : cl(c), nx(this) { }
+    ClauseListNode(ClauseListNode *n, CaseClauseNode *c)
+      : cl(c), nx(n->nx) { n->nx = this; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
@@ -841,8 +850,7 @@ namespace KJS {
 
   class CaseBlockNode: public Node {
   public:
-    CaseBlockNode(ClauseListNode *l1, CaseClauseNode *d, ClauseListNode *l2)
-      : list1(l1), def(d), list2(l2) { reverseLists(); }
+    CaseBlockNode(ClauseListNode *l1, CaseClauseNode *d, ClauseListNode *l2);
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
@@ -850,7 +858,6 @@ namespace KJS {
     virtual void processVarDecls(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
-    void reverseLists();
     ClauseListNode *list1;
     CaseClauseNode *def;
     ClauseListNode *list2;
@@ -921,8 +928,12 @@ namespace KJS {
 
   class TryNode : public StatementNode {
   public:
-    TryNode(StatementNode *b, Node *c = 0L, Node *f = 0L)
-      : block(b), _catch((CatchNode*)c), _final((FinallyNode*)f) {}
+    TryNode(StatementNode *b, CatchNode *c)
+      : block(b), _catch(c), _final(0) {}
+    TryNode(StatementNode *b, FinallyNode *f)
+      : block(b), _catch(0), _final(f) {}
+    TryNode(StatementNode *b, CatchNode *c, FinallyNode *f)
+      : block(b), _catch(c), _final(f) {}
     virtual void ref();
     virtual bool deref();
     virtual Completion execute(ExecState *exec);
@@ -936,8 +947,10 @@ namespace KJS {
 
   class ParameterNode : public Node {
   public:
-    ParameterNode(const Identifier &i) : id(i), next(0L) { }
-    ParameterNode(ParameterNode *list, const Identifier &i) : id(i), next(list) { }
+    // list is circular until cracked in FuncDeclNode/FuncExprNode ctor
+    ParameterNode(const Identifier &i) : id(i), next(this) { }
+    ParameterNode(ParameterNode *list, const Identifier &i)
+      : id(i), next(list->next) { list->next = this; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
@@ -960,8 +973,10 @@ namespace KJS {
 
   class FuncDeclNode : public StatementNode {
   public:
+    FuncDeclNode(const Identifier &i, FunctionBodyNode *b)
+      : ident(i), param(0), body(b) { }
     FuncDeclNode(const Identifier &i, ParameterNode *p, FunctionBodyNode *b)
-      : ident(i), param(p), body(b) { reverseParameterList(); }
+      : ident(i), param(p->next), body(b) { p->next = 0; }
     virtual void ref();
     virtual bool deref();
     Completion execute(ExecState */*exec*/)
@@ -969,7 +984,6 @@ namespace KJS {
     void processFuncDecl(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
-    void reverseParameterList();
     Identifier ident;
     ParameterNode *param;
     FunctionBodyNode *body;
@@ -977,14 +991,14 @@ namespace KJS {
 
   class FuncExprNode : public Node {
   public:
+    FuncExprNode(FunctionBodyNode *b) : param(0), body(b) { }
     FuncExprNode(ParameterNode *p, FunctionBodyNode *b)
-	: param(p), body(b) { reverseParameterList(); }
+      : param(p->next), body(b) { p->next = 0; }
     virtual void ref();
     virtual bool deref();
     Value evaluate(ExecState *exec);
     virtual void streamTo(SourceStream &s) const;
   private:
-    void reverseParameterList();
     ParameterNode *param;
     FunctionBodyNode *body;
   };
@@ -992,9 +1006,9 @@ namespace KJS {
   // A linked list of source element nodes
   class SourceElementsNode : public StatementNode {
   public:
-    SourceElementsNode(StatementNode *s1) { element = s1; elements = 0L; }
-    SourceElementsNode(SourceElementsNode *s1, StatementNode *s2)
-      { elements = s1; element = s2; }
+    // list is circular until cracked in BlockNode (or subclass) ctor
+    SourceElementsNode(StatementNode *s1);
+    SourceElementsNode(SourceElementsNode *s1, StatementNode *s2);
     virtual void ref();
     virtual bool deref();
     Completion execute(ExecState *exec);
-------------- next part --------------


I tried to keep gratuitous changes to the minimum, but I made some.

Sorry, I went on a bit of a "kill 0L" rampage; there's no reason to use 
0L for a pointer in C++. I couldn't help myself.

It drives me crazy how each different node class has its own name for 
the next link, from "list", to "elements", to "nx", to "next", but I 
held myself back from fixing this for now to avoid creating merging 
troubles for no reason. I think the code would be clearer if the list 
parameters were named "tail" and the next pointers were all named 
"next", but if I spend more time on KJS it will probably be to fix bugs 
or speed it up rather than on code cleanup.

     -- Darin


More information about the Khtml-devel mailing list